As decided in the staff meeting on 12. 02. 2017 (yes it took me "some" time...) we want to grant access to all command buttons and not only the first 6 by using little arrows.
This diff implements such thing. It's mainly up for discussion and agreement. Icon arrows are also missing.
Details
- Reviewers
- None
- Trac Tickets
- #4274
Select a building and a unit together (or activate the back-to-work button of a unit) and test it.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 5022 Build 8645: Vulcan Build Jenkins Build 8644: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | 490| 490| continue; | 491| 491| | 492| 492| if (state.pack.progress == 0) | 493| |- { | | 493|+ | 494| 494| if (state.pack.packed) | 495| 495| checks.unpackButton = true; | 496| 496| else | 497| 497| checks.packButton = true; | 498| |- } | | 498|+ | 499| 499| else if (state.pack.packed) | 500| 500| checks.unpackCancelButton = true; | 501| 501| else binaries/data/mods/public/gui/session/selection_panels.js | 60| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 74| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 772| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case.
Having multiple pages for it, seems the way to go on this
binaries/data/mods/public/gui/session/selection_panels.js | ||
---|---|---|
174 | maybe make a this.itemPages for the 2 | |
binaries/data/mods/public/gui/session/selection_panels_middle/unit_commands.xml | ||
15–19 | instead of hardcoding two pages, perhaps put that in a repeat aswell, and then have every page with 6 items |
Let it be uncountable pages. (Also adapt XML and js, so that we can show all possible 13 buttons we currently have)
binaries/data/mods/public/gui/session/selection_panels.js | ||
---|---|---|
125–126 | maybe these should be moved to the top of the file (not sure for this file), also some jsdocs perhaps | |
187 | the number of pages could fluctuate over time, and can be 3 to 2 so I guess you mean if (numberOfPages < g_OpenCommandPage) g_OpenCommandPage = max(1, numberOfPages); (not sure if that 1 is required) | |
190–191 | if there are no pages, at least one of them will bug I guess the max(1,foo) from above should be removed and check for <=1 here | |
binaries/data/mods/public/gui/session/selection_panels_middle/unit_commands.xml | ||
15 | they end up as black squares now, some arrow would look better (I know there are space problems...) | |
16 | We rely here on the onTick/onSimupdates for updating the buttons and those can take an arbitrary amount of time, instead of doing it directly by updateSelectionDetails |
binaries/data/mods/public/gui/session/selection_panels.js | ||
---|---|---|
125–126 | Thought I leave them her, because they are only used here... | |
187 | true, I still had only 2 pages in mind ^^ | |
190–191 |
no pages <=> no items => panel won't be shown
which max? | |
binaries/data/mods/public/gui/session/selection_panels_middle/unit_commands.xml | ||
15 |
See the summary of this diff:
|
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | 490| 490| continue; | 491| 491| | 492| 492| if (state.pack.progress == 0) | 493| |- { | | 493|+ | 494| 494| if (state.pack.packed) | 495| 495| checks.unpackButton = true; | 496| 496| else | 497| 497| checks.packButton = true; | 498| |- } | | 498|+ | 499| 499| else if (state.pack.packed) | 500| 500| checks.unpackCancelButton = true; | 501| 501| else binaries/data/mods/public/gui/session/selection_panels.js | 60| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 74| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 772| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case.
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | 497| 497| continue; | 498| 498| | 499| 499| if (state.pack.progress == 0) | 500| |- { | | 500|+ | 501| 501| if (state.pack.packed) | 502| 502| checks.unpackButton = true; | 503| 503| else | 504| 504| checks.packButton = true; | 505| |- } | | 505|+ | 506| 506| else if (state.pack.packed) | 507| 507| checks.unpackCancelButton = true; | 508| 508| else binaries/data/mods/public/gui/session/selection_panels.js | 60| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 74| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 779| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case.
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/selection_panels.js | 497| 497| continue; | 498| 498| | 499| 499| if (state.pack.progress == 0) | 500| |- { | | 500|+ | 501| 501| if (state.pack.packed) | 502| 502| checks.unpackButton = true; | 503| 503| else | 504| 504| checks.packButton = true; | 505| |- } | | 505|+ | 506| 506| else if (state.pack.packed) | 507| 507| checks.unpackCancelButton = true; | 508| 508| else binaries/data/mods/public/gui/session/selection_panels.js | 60| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 74| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 779| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case.
True, there is https://github.com/0ad/0ad/blob/master/binaries/data/mods/public/art/textures/selection/arrow/128x128.png
I'll rotate that and try it out.
don't you think that buttons are too small to click. it could be the same size with the others and it could be a "forward" icon instead of narrowed play button. loves and kisses. xoxox
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | 497| 497| continue; | 498| 498| | 499| 499| if (state.pack.progress == 0) | 500| |- { | | 500|+ | 501| 501| if (state.pack.packed) | 502| 502| checks.unpackButton = true; | 503| 503| else | 504| 504| checks.packButton = true; | 505| |- } | | 505|+ | 506| 506| else if (state.pack.packed) | 507| 507| checks.unpackCancelButton = true; | 508| 508| else binaries/data/mods/public/gui/session/selection_panels.js | 60| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 74| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 779| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case.
Link to build: https://jenkins.wildfiregames.com/job/differential/37/display/redirect
Agree.
But I am no artist and this is the best I could reach for, so an artist (hint, hint ;P) should do a better icon ;)
I can help to turn that ugly button into a better-looking button if you give me the source file.
I need the png file of one of those icons. I'm asking to understand the size of icons then I can come up with some ideas.
I'm not sure if they look exactly like this in the menu because I added them on ps. Anyway, we can discuss their shapes and colors. I think it should be a "forward button" instead of a "play button". That's why the shape is like this and the color should be in an attention color like yellow, orange and red. It needs to say "there are more buttons" clearly. Well, I can work on it after we discuss it.
There is too much deformation here. I think the icon needs more space to make it wider if it's possible.
I can offer this
I can also probably make highlighted versions if needed.
binaries/data/mods/public/gui/session/selection_panels_middle/unit_commands.xml | ||
---|---|---|
23 | Since the OOP rewrite, maybe this can be done nicely in the JS? |
binaries/data/mods/public/gui/session/selection_panels_middle/unit_commands.xml | ||
---|---|---|
23 | Good question |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (no-multi-spaces): | | Multiple spaces found before 'state'. |----| | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | 317| 317| if (!g_AvailableFormations.has(unitEntStates[0].player)) | 318| 318| g_AvailableFormations.set(unitEntStates[0].player, Engine.GuiInterfaceCall("GetAvailableFormations", unitEntStates[0].player)); | 319| 319| | 320| |- return g_AvailableFormations.get(unitEntStates[0].player).filter(formation => unitEntStates.some(state => !!state.identity && state.identity.formations.indexOf(formation) != -1)); | | 320|+ return g_AvailableFormations.get(unitEntStates[0].player).filter(formation => unitEntStates.some(state => !!state.identity && state.identity.formations.indexOf(formation) != -1)); | 321| 321| }, | 322| 322| "setupButton": function(data) | 323| 323| { | | [NORMAL] ESLintBear (space-before-function-paren): | | Unexpected space before function parentheses. |----| | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | 780| 780| addResearchToQueue(data.item.researchFacilityId, t); | 781| 781| })(tech); | 782| 782| | 783| |- button.onPressRight = (t => function () { | | 783|+ button.onPressRight = (t => function() { | 784| 784| showTemplateDetails( | 785| 785| t, | 786| 786| GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv); binaries/data/mods/public/gui/session/selection_panels.js | 50| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 61| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 748| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2522/display/redirect
binaries/data/mods/public/gui/session/selection_panels_middle/unit_commands.xml | ||
---|---|---|
23 | Hmm I don't see how this could benefit from doing it in js only. |
Use icons from Stan. No extra icon definition. Remove change crept in from other diff.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (no-multi-spaces): | | Multiple spaces found before 'state'. |----| | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | 317| 317| if (!g_AvailableFormations.has(unitEntStates[0].player)) | 318| 318| g_AvailableFormations.set(unitEntStates[0].player, Engine.GuiInterfaceCall("GetAvailableFormations", unitEntStates[0].player)); | 319| 319| | 320| |- return g_AvailableFormations.get(unitEntStates[0].player).filter(formation => unitEntStates.some(state => !!state.identity && state.identity.formations.indexOf(formation) != -1)); | | 320|+ return g_AvailableFormations.get(unitEntStates[0].player).filter(formation => unitEntStates.some(state => !!state.identity && state.identity.formations.indexOf(formation) != -1)); | 321| 321| }, | 322| 322| "setupButton": function(data) | 323| 323| { | | [NORMAL] ESLintBear (space-before-function-paren): | | Unexpected space before function parentheses. |----| | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js | 780| 780| addResearchToQueue(data.item.researchFacilityId, t); | 781| 781| })(tech); | 782| 782| | 783| |- button.onPressRight = (t => function () { | | 783|+ button.onPressRight = (t => function() { | 784| 784| showTemplateDetails( | 785| 785| t, | 786| 786| GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv); binaries/data/mods/public/gui/session/selection_panels.js | 50| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 61| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 748| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2523/display/redirect
It's a bit of a pain to quickly click on them now, perhaps show five at a time and increase the size of the arrows?
The benefit of the OOP is that e.g. the diff to scroll through the loading screen tips can partly use the same code. But @elexis would be way better at explaining that ^^
binaries/data/mods/public/gui/session/selection_panels.js | ||
---|---|---|
133 | +\n. |
Tested and functional, I would agree with Freagarach that perhaps only showing 5 and having bigger arrows is easier to click though. Keep showing 6 in the regular case.
Hmm, I wanted to stay with 6 command buttons per page, so that you don't have to use the second page too often...
The benefit of the OOP is that e.g. the diff to scroll through the loading screen tips can partly use the same code. But @elexis would be way better at explaining that ^^
Hmm, and when we have more than 10 buttons we have 5 4 and then the rest?
For what it's worth I've been considering this change instead: https://github.com/wraitii/ui_mod/blob/master/ex_unit.jpg