Opened 7 years ago
Last modified 12 months ago
#3987 new defect
Implement options to randomly choose from a subset of civilisations during gamesetup
Reported by: | elexis | Owned by: | |
---|---|---|---|
Priority: | Should Have | Milestone: | Backlog |
Component: | UI – Game setup | Keywords: | |
Cc: | O.Davoodi, s0600204, Krinkle | Patch: |
Description (last modified by )
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.)
While the original ticket has been fixed in https://trac.wildfiregames.com/changeset/26954
it would still be nice to have advanced options to choose randomly from a specified subset of civilizations.
As this ticket contains quite some discussion and information, I'm leaving it open for the future.
Attachments (8)
Change History (31)
by , 7 years ago
Attachment: | trulyrandomcivs.js.patch added |
---|
comment:1 by , 7 years ago
Keywords: | review patch added |
---|---|
Milestone: | Backlog → Alpha 21 |
Summary: | Chosing a random civilization should mean that all civs have the same chance of being picked → [PATCH] Chosing a random civilization should mean that all civs have the same chance of being picked |
comment:2 by , 7 years ago
comment:3 by , 7 years ago
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.
comment:4 by , 7 years ago
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 }
comment:5 by , 7 years ago
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.
comment:6 by , 7 years ago
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
comment:7 by , 7 years ago
That sounds fine to me (especially not having it in the gamesetup but in the settings). Thanks for elaborating :)
comment:8 by , 7 years ago
Cc: | added |
---|---|
Keywords: | simple review removed |
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.
comment:9 by , 7 years ago
Keywords: | review added |
---|
by , 7 years ago
Attachment: | civselection_and_randomcivoption.png added |
---|
I have updated s0600204's civ selection menu and added the Truly Random Civs option to it, is this what is wanted?
comment:10 by , 7 years ago
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).
comment:11 by , 7 years ago
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.
by , 7 years ago
Attachment: | civselection_randomcivoption.patch added |
---|
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.
comment:12 by , 7 years ago
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
by , 7 years ago
Attachment: | civselection_randomcivoptionWIP.patch added |
---|
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.
comment:13 by , 7 years ago
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.
comment:14 by , 7 years ago
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.
comment:15 by , 7 years ago
Keywords: | review removed |
---|
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.
by , 7 years ago
Attachment: | civselection_randomcivoptionWIP_v3.patch added |
---|
Still needs fixes and code quality improvements.
by , 7 years ago
Attachment: | civselection_randomcivoption_v3.1.patch added |
---|
comment:16 by , 7 years ago
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.
comment:17 by , 7 years ago
Milestone: | Alpha 21 → Backlog |
---|
comment:18 by , 4 years ago
Component: | UI & Simulation → Game setup |
---|
Move tickets to Game Setup
as UI & Simulation
got some sub components.
comment:19 by , 4 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
comment:20 by , 3 years ago
An updated version of the civselection dialog is found here https://github.com/JustusAvramenko/delenda_est/commit/bd1e73bf133674e790aa9f686f4b41e2ccba0a43
Screenshots at https://wildfiregames.com/forum/index.php?/topic/25206-civilization-selection-screen/
For the problem originally described in the ticket: The easiest solution to please everyone could be to not have ["Random", "Civ1", ... , "CivN"] choices for the dropdown, but ["Random Civ", "Random civ of random Culture", "Civ1", ... , "CivN"]. And to options.json one could add a config option that determines which of these N+2 items is chosen by default if the player can control over their own slot (refs #3806). So players can chose romans by default, or random civ, or random civ of a random culture. This could be integrated with the civ dialog as well (providing the same N+2 choices, but with images and text).
comment:23 by , 12 months ago
Description: | modified (diff) |
---|---|
Keywords: | patch removed |
Summary: | [PATCH] Chosing a random civilization should mean that all civs have the same chance of being picked → Implement options to randomly choose from a subset of civilisations during gamesetup |
programming.json