#1580 closed enhancement (fixed)
[PATCH] Change player color in the gamesetup
Reported by: | O.Davoodi | Owned by: | Stan |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 19 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
Discussion here http://www.wildfiregames.com/forum/index.php?showtopic=16318
Attachments (17)
Change History (43)
comment:1 by , 11 years ago
Milestone: | Alpha 12 → Backlog |
---|
comment:2 by , 11 years ago
Owner: | removed |
---|
comment:3 by , 10 years ago
Owner: | set to |
---|
by , 10 years ago
Attachment: | customColorSupport.patch added |
---|
comment:4 by , 10 years ago
Keywords: | review patch added |
---|---|
Milestone: | Backlog → Alpha 15 |
Based on the previous work that Spahbod posted in the forums, I’m attaching a patch which in addition does the following:
- Keeps color display names in a string of their own, in preparation for the internationalization feature (#67).
- Sorts colors in the drop-down list by color.
Although I was planning on doing so, I refrained from including ancient-themed colors. I think that needs more discussion (even if we all agree on doing it, we still need to agree on the RGB and display name of the values to be added), and I’ll try to bump the forum thread about it.
Adding new colors is as easy as editing the relevand JavaScript object in code. In the future, we could even move such data into a separated .json file if needed.
Note also that the implementation is prepared to deal with a color defined in simulation/data/player_defaults.json not being available in the JavaScript object that defined colors and color names in the GUI code. This is to keep simulation and gui code separated, as they should not depend in one another. When this occurs (a color is unknown in the GUI), the drop-down list shows empty, and lets you either keep the current, unknown color, or change to another color; if you change, you can not select back any unknown color. (you’ll probably need to actually read the code to understand this, just look for “-1” in the patch)
comment:5 by , 10 years ago
Summary: | Adding the ability to change player colour in game setup → [PATCH] Adding the ability to change player colour in game setup |
---|
comment:6 by , 10 years ago
Owner: | removed |
---|
comment:7 by , 10 years ago
Milestone: | Alpha 15 → Alpha 16 |
---|
comment:8 by , 10 years ago
Keywords: | color gamesetup review removed |
---|
Can you make this patch work for skirmish maps too?
And the patch causes some problems when using a low resolution. The colour picker should be made smaller. Maybe only showing coloured squares instead of colour names?
comment:9 by , 10 years ago
Milestone: | Alpha 16 → Alpha 17 |
---|
comment:10 by , 10 years ago
Milestone: | Alpha 17 → Backlog |
---|
comment:11 by , 9 years ago
Owner: | set to |
---|
by , 9 years ago
WIP patch, it's currently broken for some reason, help is appreciated. I can't get the drop down to appear
comment:12 by , 9 years ago
Note that the json file will be removed, I added it in the patch, cause I forgot to delete it. It actually use default_players to get the colors.
comment:13 by , 9 years ago
- Please create the patch relatively to the trunk directory, so that we can apply the patch from there
settings.js
PlayerColors
instead ofPlayer_Colors
- the code was updated
gamesetup.js
- ++o instead of o++. I'd prefer
n
ori
overo
. But even better would be to remove that loop and just use array.find(...).length if possible.
getIndexOfColor
don't need to dovar playerColorsSorted = getPlayerColorsSorted();
every cycle, only once. replace the for loop by a call tofindIndex
rgbToHsl
- exact same function as inlobby.js
. we should annoy the devs with a ticket that movesrgbToHsl
andhslToRgb
to a new filecommon/color.js
(the other color functions from lobby.js are almost all baloney, for example the rainbow colored pony-nicknames, and must be deleted in #3383 or so instead of being moved)
getPlayerColorsSorted()
- early return
[]
instead ofnew Array()
- replacing for with map might be possible
- if the block has only one line, the brackets can be removed (unless neede for aesthetics)
let
instead ofvar
in nested blocks so that theoretically a bit of performance can be savedouterloop
: try to avoid jumping to labels, I bet we can use a map or something similar for the inner loop and omit the problem that way
copyRgbValuesFromColorIntoColor
- whitespace
gamesetup.xml
- a tab missing
- the position/size you gave the color picker is almost identical to the position of the player assignment dropdown. That might be why the former is invisible (but shows the tooltip when hovering) and why the latter seems disabled. The values are "left top right bottom". The "right" value of the left element must be smaller than the "left" value of the right element, etc. Where do you want to palce the color picker? Left of the playername? The xml should be moved to the correct location too.
Some rtfm's: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf and the other array functions linked there https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions http://trac.wildfiregames.com/wiki/Coding_Conventions
by , 9 years ago
Attachment: | 1580.4.diff added |
---|
by , 9 years ago
Attachment: | 1580.5.diff added |
---|
by , 9 years ago
Attachment: | 1580.7.diff added |
---|
comment:14 by , 9 years ago
Milestone: | Backlog → Alpha 19 |
---|
- hardcore cleanup
- gaia color not selectable anymore
- colors are actually used ingame after selecting them
TODO:
- only host can chose colors
- dropdown element should list only colors that are not picked yet
- discovered a dropdown list GUI bug. looks like the width of the scrollbar is subtracted even if the scrollbar doesn't exist. Might also have something to do with the xml / width.
by , 9 years ago
Attachment: | 1580.8.diff added |
---|
comment:15 by , 9 years ago
Keywords: | review added |
---|
- As agreed on IRC, it's better for now that only the host can change colors, so that there is no fight about it.
- There is no need for not showing colors, since we swap between them when they are being used.
- Only the gui bug remains on certain resolutions.
comment:16 by , 9 years ago
Description: | modified (diff) |
---|---|
Summary: | [PATCH] Adding the ability to change player colour in game setup → [PATCH] Change player color in the gamesetup |
by , 9 years ago
Attachment: | l10n_longestStrings.7z added |
---|
Mock language that contains the longest strings found in the translations of r17026.
comment:17 by , 9 years ago
As leper mentioned on IRC:
- We should have a comment near that
splice
, like// Gaia color not selectable
- Remove that last space in l387 of
gamesetup.js
- The column that has "Player 1" to "Player 8" in it might be too small to fit for all translations. Make sure that it fits for all translations. You could check with the mock language above, which was generated using http://trac.wildfiregames.com/wiki/Implementation_of_Internationalization_and_Localization#LongStringsLocale. leper said there might be some false positives (unfinished / wrong translations or something). The translation files should be edited in that case and the script run again.
by , 9 years ago
Attachment: | 1580.10.diff added |
---|
Version Without Space as it breaks on smaller resolutions this version still needs the c++ patch
by , 9 years ago
Attachment: | 1580.10.Workaround.diff added |
---|
Same as previous, but without the need for c++
comment:18 by , 9 years ago
- Found a bug: Select an unused color and increase the number of players. That color will be used twice. Fixed it by fixing the object instead of blindly reseting the colors.
- Moved the logic to a new function as
initMain()
is crowded. - Updated
gamesetup.xml
to use proper positions and hide the colorpicker caption too for scenario maps. - For 1600x1200 that gui bug #3415 still applies, even with scrollbar="false" (not a dealbreaker imho).
- Updated that gaia comment.
- Fixed a missing semicolon.
Patch by all three of us.
by , 9 years ago
Attachment: | 1580.11.diff added |
---|
by , 9 years ago
Attachment: | 1580.12.diff added |
---|
comment:20 by , 9 years ago
- Drops support for colorpicking on skirmish maps, as they can define custom colors, which would introduce the "bug" of having players with seemingly identical colors. Reproduce with prior versions:
- select skirmish map median oasis (4)
- make player 3 teal
- => two players are teal, but not the exact same teal! argh!!11
rgb(50, 170, 170)rgb(64, 163, 191)
- Fixed edge-case of the "duplicate color" bug described in comment:18 by processing the array in reverse order
- New GUI style as proposed by mimo (see http://www.imagebam.com/image/856651436341384)
- Removed the grey overlay sprite of the dropdown list
- Resized the element, so that the rectangle is invisible unless opening the list
- Some more simplifications and cleanup
comment:21 by , 9 years ago
I'm not very fond of the new layout but if the function is here I'm fine.
For skirmish maps see comment 9.
by , 9 years ago
Attachment: | 1580.13.diff added |
---|
There you go, skirmish supported again. Colors that are not identical to the colors in player_defaults.json
will be replaced with the closest match!
by , 9 years ago
Attachment: | 1580.14.patch added |
---|
Ensures player colors are unique after matching to player_defaults.json
.
Patch based on the work started by Spahbod.