[PATCH] Add "Random" option for selecting civilizations in match setup.

[Spahbod]

All of the games have such option when player wants to choose a civ from dropbox.

[deebee]

Hello, I'm new here. I tried working on this ticket as it looked easier to tackle.

The civilization is chosen based on the map selected; there is no option to change the civilization. To add the "Random" option, I would assume that the user should be able to choose the civilization in the first place. We could provide the civilization dropdown with defaults based on the maps; and then we could add the random civilization option. Is this planned?

[Spahbod]

This is for random maps not scenarios. Part of it is in function initCivNameList() I think.

[historic_bruno]

I like this idea. For now it should be only random maps, though I've heard we might allow changing civs on secnarios at some point.

[deebee]

Alright, I've attached a patch to add this feature. It should also work if in future 'scenarios' were to allow selecting random civs.

[Spahbod]

It works well. But there is something:

Random is better to be the first choice. But The logic won't need to be changed very much as it only needs you to change the function push to unshift and use this one for the line 735:

 	                g_GameAttributes.settings.PlayerData[i].Civ = civBox.list_data[Math.floor(Math.random()*(civBox.list_data.length-1))+1];

And why did you change the color? It looks unselectable. Also you can set the default civ to the Random option as other games.

[deebee]

I agree. Random would be better as first choice and a different color would be good. I was trying to keep it consistent with the "unassigned" option of "Player assignment/placement" dropdown. I got the impression that it was a random picking of AI. What is that option though?

[Spahbod]

Different color is not bad by itself. But that color makes it look like unselectable. I suggest you use some bright color like orange or green or anything that looks like selectable.

About the last part I mean that for now the default civ is Hellenes. It is better it would be Random instead.

[historic_bruno]

Replying to deebee:

I got the impression that it was a random picking of AI. What is that option though?

Unassigned means no AI or human player in that case. So I agree a color other than grey or green would be best :)

I would make the random civ code lower case to match the convention for other civs ("hele", "celt", etc.).

The way you're assigning the random civ will only work for single player games, because it's in the non-networked path of LaunchGame. It seems better to do the random civ assignment before that branching, and we wouldn't have to worry about network synchronization because the host dictates the attributes for all players.

Also it breaks the client UI update, because there's no "Random" in g_CivData. You could special case the code in onGameAttributesChange, it's only for display purposes.

One thing to consider, this doesn't work at all with autostart because that mode bypasses the game setup UI. This would be a problem if we allowed random civs in scenarios or if someone explicitly specified the player civs for a random map (I think autostart uses a hardcoded civ right now for random maps, but that's likely to change). So maybe we should do the random player assignment in simulation\helpers\Player.js instead? This is more of a future concern, but if we can fix it now, why not? :)

In case it's not clear, we specify default civs in simulation\data\player_defaults.json. The reason I mention that is it's an easy way to make "Random" the default civ, if we choose to do that.

[deebee]

Random Civ Option - put it as the first option, text color changed to orange, works on client UI, works on multiplayer and made it the default civ

[deebee]

I've corrected few bugs as mentioned above. I haven't got a chance to work on autostart part. It'll be next tuesday before I'll be able to work on it though. Let me know if any more concerns in this patch though.

[Spahbod]

Is it really necessary to have the option in autostart? Autostart is mainly intended for developers not real players.

[deebee]

This will be a bug for autostart later, if it's not fixed now.

[historic_bruno]

Replying to Spahbod:

Autostart is mainly intended for developers not real players.

Well both I think, anyone who doesn't want to go through the game setup menus to play a game. It's not a priority to add new features to autostart, but we shouldn't break it either :)

[deebee]

Merges the changes from previous patch's.

[deebee]

Unassigned means no AI or human player in that case. So I agree a color other than grey or green would be best :)

The color is now orange. It can updated if needed.

I would make the random civ code lower case to match the convention for other civs ("hele", "celt", etc.).

The random civ code is now 'random'.

The way you're assigning the random civ will only work for single player games, because it's in the non-networked path of LaunchGame. It seems better to do the random civ assignment before that branching, and we wouldn't have to worry about network synchronization because the host dictates the attributes for all players.

Also it breaks the client UI update, because there's no "Random" in g_CivData. You could special case the code in onGameAttributesChange, it's only for display purposes.

I've now put the assigning outside the non-networked path. I did test this and seems to work fine. Also, the client can now see the 'Random' civ selected.

One thing to consider, this doesn't work at all with autostart because that mode bypasses the game setup UI. This would be a problem if we allowed random civs in scenarios or if someone explicitly specified the player civs for a random map (I think autostart uses a hardcoded civ right now for random maps, but that's likely to change). So maybe we should do the random player assignment in simulation\helpers\Player.js instead? This is more of a future concern, but if we can fix it now, why not? :)

I've changed SetCiv? in binaries/data/mods/public/simulation/components/Player.js to store a randomly selected civ whenever 'random' is passed. Also, I've added a new function, Script_GetRandomCiv() to /source/simulation2/system/ComponentManager.h exposed to the scripts. This will load all civilizations (LoadCivData?()) and fetch a random civ from it (getRandomInt()). The GetRandomCiv? function is used in SetCiv?(). I tested this by changing civ's in scenario map Barcania to 'random'; worked fine but I think the random function is deterministic, may be it's not. Also, for autostart with random maps, I've changed a/source/ps/GameSetup/GameSetup.cpp to pick a random map instead of "hele" like it always does.

In case it's not clear, we specify default civs in simulation\data\player_defaults.json. The reason I mention that is it's an easy way to make "Random" the default civ, if we choose to do that.

I have updated player_defaults.json to use 'random' as the default civilization.

[Philip]

I don't think duplicating the random civ-selection logic into Player.js+ComponentManager.cpp just for autostart is worth the complexity, and I don't think it's desirable behaviour: autostart is intended primarily for developer testing and it needs to give consistently reproducible behaviour, so we could use an RNG with a constant seed but we might as well just hardcode it to always use 'hele' when the map allows random civs (since that's just as good as any other constant random selection). (Then we could add some extra command-line options like "-autostart-civ=1:celt" to override the default, but that's an independent task.)

In that case, I think using just the changes to gamesetup.js and player_defaults.json (but does that cause any problems for the player selection UI in Atlas?) would be fine.

[historic_bruno]

Replying to Philip:

In that case, I think using just the changes to gamesetup.js and player_defaults.json (but does that cause any problems for the player selection UI in Atlas?) would be fine.

Continuing our conversation from IRC, I agree it would be fine to leave out the autostart bit. Even though some people would like random civs in autostart for convenience (including myself :)), that code is messy and the more we add, the worse it gets.

Actually I didn't know we had plans for some other GUI-based "quick start" mode, I thought autostart was it. We need a ticket for that.

[Spahbod]

I tested the codes. If we don't want to add it to autostart, atlas will crash. I think we should define the default civ in gamesetup.js not in player_defaults.js.

[ben]

In 11778:

Adds random civ option to game setup, based on patch by deebee. Fixes #1082.

[historic_bruno]

Thanks, sorry it took so long :(

[fabio]

It would be nice to have random as the default civ for all, rather that the first 4 civs in order.

Especially since these are "random" maps :)