Page MenuHomeWildfire Games

Session: Show more command buttons
Needs ReviewPublic

Authored by Imarok on Jan 28 2018, 10:52 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#4274
Summary

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.

Test Plan

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 BuildJenkins
Build 8644: arc lint + arc unit

Event Timeline

Imarok created this revision.Jan 28 2018, 10:52 PM
Vulcan added a subscriber: Vulcan.Jan 28 2018, 11:43 PM

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.
bb added a subscriber: bb.Jan 30 2018, 12:48 AM

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

Imarok updated the Trac tickets for this revision.Jan 30 2018, 4:52 PM
Imarok updated this revision to Diff 5592.Jan 30 2018, 5:04 PM

Let it be uncountable pages. (Also adapt XML and js, so that we can show all possible 13 buttons we currently have)

bb added inline comments.Jan 30 2018, 5:55 PM
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

Imarok marked 4 inline comments as done.Jan 30 2018, 6:24 PM
Imarok added inline comments.
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

if there are no pages, at least one of them will bug

no pages <=> no items => panel won't be shown

I guess the max(1,foo) from above should be removed and check for <=1 here

which max?

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...)

See the summary of this diff:

This diff implements such thing. It's mainly up for discussion and agreement. Icon arrows are also missing.

Imarok updated this revision to Diff 5593.Jan 30 2018, 6:24 PM
Imarok marked 4 inline comments as done.

See the comments.

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.

(ugly) arrows:


Needed images:

Owners added a subscriber: Restricted Owners Package.Feb 3 2018, 10:28 PM

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.
Stan added a subscriber: Stan.Feb 3 2018, 11:36 PM

There is no arrow in the game at all ?

Imarok added a comment.Feb 4 2018, 6:53 PM
In D1269#52139, @Stan wrote:

There is no arrow in the game at all ?

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.

In D1269#52132, @Imarok wrote:

(ugly) arrows:



Needed images:

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

Imarok updated this revision to Diff 5821.EditedFeb 17 2018, 10:38 PM

Update images

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

Stan added a comment.Feb 18 2018, 1:26 PM

That doesn't look good :/

In D1269#53535, @Stan wrote:

That doesn't look good :/

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 ;)

In D1269#53945, @Imarok wrote:
In D1269#53535, @Stan wrote:

That doesn't look good :/

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.

In D1269#53945, @Imarok wrote:
In D1269#53535, @Stan wrote:

That doesn't look good :/

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.

Source file of what?

In D1269#53972, @Imarok wrote:
In D1269#53945, @Imarok wrote:
In D1269#53535, @Stan wrote:

That doesn't look good :/

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.

Source file of what?

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.

In D1269#53972, @Imarok wrote:
In D1269#53945, @Imarok wrote:
In D1269#53535, @Stan wrote:

That doesn't look good :/

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.

Source file of what?

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.

They are uploaded on this page. Just look a bit below this post...

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.

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.

In D1269#54037, @Imarok wrote:

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.

It might be nice to also be able to scroll to change the "page"?