[PATCH] Chosing a random civilization should mean that all civs have the same chance of being picked

[elexis]

Since r12704, the current gamesetup code in launchGame() first picks a random culture (like hellenic or celtic) and then picks a random civ of that culture.

That however means that some civs are over/underrepresented. For example the chance to get Ptolemies is three times bigger than the chance to get Athens (as there are three hellenic civs).

Here the cultures of the available civs (binaries/data/mods/public/simulation/data/civs):

spart.json:	"Culture":"hele",
mace.json:	"Culture":"hele",
athen.json:	"Culture":"hele",
gaul.json:	"Culture": "celt",
brit.json:	"Culture": "celt",
rome.json:	"Culture": "rome",
sele.json:	"Culture":"sele",
ptol.json:	"Culture":"ptol",
maur.json:	"Culture": "maur",
pers.json:	"Culture": "pers",
cart.json:	"Culture": "cart",
iber.json:	"Culture": "iber",

In my humble opinion, this should be reverted, so that all civs have the same chance of appearing. (Just because they share some of the same buildings and stats doesn't mean they should be picked less often.)

[elexis]

• Remove those braces in L1255 L1260 as they are not needed anymore.
• The comments are not really useful anymore in L1246 and L1253, so remove them too
• Don't forget to add yourself to programming.json

[mimo]

Is this patch really something we want ? i prefer the current way which gives more variety to games with a reduced chance to have opponents with buildings with the same look. Maybe a solution to please everyone would be to add a player option to choose between the current way (based on culture, which i think should be the default) and the fully random one.

[Sandarac]

Adds check box in More Options

[fatherbushido]

From the formal point of view : L1267 L1269, you can/must remove the { }. Moreover, you can just remove the else part and just put L1264 after the }

[elexis]

But we also need a clear design decision on whether we should go with the first, second or original approach before you do any further work that might become rejected.

As stated above, mimo prefers the culture distinction, while I'd prefer to give all civs the same chance and not complicate this (Would players actually understand what that option is used for? Wouldn't we spam the options area a bit?). I wonder where we should get a clear decision from, since most developers are busy / not-available right now and those that are disagree or don't have a strong opinion x) So I wonder whether we should make a forum thread about this and ask the actual players what they prefer.

Here you can read the chatlog (to 17:23 to 18:34): ​http://irclogs.wildfiregames.com/2016-05-23-QuakeNet-%230ad-dev.log

Of course you are always welcome to join #0ad-dev for asking us anything directly: ​http://irc.lc/quakenet/0ad-dev/

Thanks for your patience.

[mimo]

To answer elexis remarks first, i think that for players to understand what all this means, it is only up to us to give the right tooltips. And i believe the final game should be much more customizable than it is now, with a more complete option panel to adapt it to the player taste. But i agree with him to advice you to drop by on IRC to discuss it a bit before doing more work on this ticket.

Then why is the culture choice relevant: we already have 3 hele civs which share most of the buildings, and if you use a mod like delenda, it even has 5 such hele civs. Of course, a core player who has spent time to compare the different unit stats will see that they play a bit differently, but a casual player who cares more about the immersion and the look rather than the unit stats could have the impression to always play the same civ. So having a random culture by default for new players looks much better to me. Then the core players can change this option to random civ once they know the game better (although after quite a long time, I still prefer a random culture).

Finaly going back to the patch, I think this option should be made more persistent, as it is something a player will rarely/never change. We should rather add it in binaries/data/config/default.cfg with a dropdown in gui/options/options.json. If doing that, we could regroup all options affecting gamesettings: persistmatchsettings could be changed to something like gamesettings.persistsettings and we could add gamesettings.civchoice

[elexis]

That sounds fine to me (especially not having it in the gamesetup but in the settings). Thanks for elaborating :)

[elexis]

As noticed by leper, this could be solved on a broader scale. The host could select the civilizations that the randomization would chose from.

A checkbox would be added to that dialog in order to please both preferences with regards to culture-based randomization.

There is a constructionsite for alpha 18 by s0600204 doing something like that: ​https://wildfiregames.com/forum/index.php?/topic/19724-design-civillization-name/#comment-306957 ​https://github.com/s0600204/0ad-civselection-mod

The new dialog would be the right place to move the disputed checkbox.

Taking out of the review queue as the more-options window is already filled with many options and it's likely an option which will be used rarely. A general option (main menu -> settings) should be considered a temporary solution in case noone works on the real deal.

[Sandarac]

I have updated s0600204's civ selection menu and added the Truly Random Civs option to it, is this what is wanted?

[elexis]

That certainly looks interesting, but to judge we need the patch (and maybe s0600204 wants to update his repository too if we don't accept it for any reason).

[s0600204]

I'll have a look at a patch if one's provided, but I'm not merging it into that repo. For the simple reason that I migrated the mod to a branch on my fork of 0AD on GitHub months ago (​https://github.com/s0600204/0ad/tree/civselect) for ease of maintenance and ​development.

Also, if you hadn't already noticed, please note that the civselect repo/branch alters the Culture attribute for all civs, so as to aid grouping on the page. Thus, the civ-to-culture list in the description of this ticket is not true if you have the civselect mod/branch active.

[Sandarac]

All credit goes to s0600204 for the civ selection menu. If the user has Random Cultures enabled (default), then "Random All" will select random civs from random cultures, otherwise truly random.

[mimo]

Thanks for the new patch. It seems to me that it has a lot of potentials, but also is not easy to use. Some additional tooltips or a better disposition of the controls could help.

• I would have expected that the civ tooltip (displayed on the right) is also visible (as a true tooltip maybe to differentiate it from the selected civ) on mouseOver and not only onClick (having to click on each icon is a pain).
• In addition, as new players are not used to the civ icon and do not recognize them, having the name written below the icon would help.
• why is the "select all" checkbox only visible when "ungrouped" ?.And the position of the different checkboxes is not clear. I would have expected that the "random cultures" is dependent on the "select all" one (i.e. disabled when the select all is not on), and grouped together and not as now on different lines and columns which give the impression they have nothing to do together).
• if i select the european civs for example (in the group by geography) and then switch to ungrouped or group by culture views, i would have expected the european civs to be shown as selected

[Sandarac]

Fixes issues pointed out by mimo, and some other issues, but the code could be cleaner. Having the civ/grouping name displayed on the right panel and as a tooltip should be enough. Adds double click functionality for selecting civs/groupings.

[Sandarac]

More Fixes.

[wraitii]

Some comments on what the patch should do: -You need to change the reset icon -Clicking on the gear is more difficult than clicking on the dropdown and this is annoying. I'd suggest making the whole thing a button.

-I really like the sorting by culture, but there's obviously quite a bit of wasted space. Would be nice to improve that view and make it default imo (see attached screenshot).

-Clicking on several icons using shift + click should select both and make the choice random between those. -Drag and dropping should probably select what's inside. -Clicking on the currently selected civ should imo deselect it (thus reselecting everything) instead of this very clumsy "select all". -The "group by culture" button is very impractical. My screenshot solution would allow doing it differently.

A code comment: -not a fan of your culture naming convention.

[wraitii]

​http://imgur.com/a/UlmDl

Imgur album with screenshots of what I suggest. Here I grouped by culture, but could switch to region. My suggestion is that you can select a group (which would show something like my big successor badge here) or specifically a civilization in a group (hovering over the group should fade it somewhat and allow choosing). Clicking anywhere on the group but not the civ selects the group.

You can then mix and match however you want and the screen remains rather nice and uncluttered.

nb those are photoshop mockups.

[Itms]

This has been reviewed by wraitii and I agree with all his comments. Additionally there is a glitch: I can't untoggle the "Select all" radio button.

I like the idea though, thanks for your work!

For you next submission, please take a look at SubmittingPatches, since we are changing some things in our review process.

[Sandarac]

Still needs fixes and code quality improvements.

[elexis]

Hey Sandarac,

since this is huge patch and the ones reviewing certainly don't want to reread the entire patch every revision, you should start using github for that. Just create an account over there, open ​https://github.com/0ad/0ad/ and click on fork, create a branch for this ticket, clone your fork to your harddrive, checkout that branch, apply the patch, commit it and push it upstream. Then you can do incremental commits and if you need to fix some typos, one only has to read those lines you changed.

[Imarok]

Move tickets to Game Setup as UI & Simulation got some sub components.