Page MenuHomeWildfire Games

Make tab actually toggle status bars
ClosedPublic

Authored by causative on May 22 2017, 3:27 AM.

Details

Summary

from default.cfg:

session.showstatusbars = Tab ; Toggle display of status bars

The current behavior of session.showstatusbars is to show them only as long as the key (by default, tab) is held down. This is not toggling. Toggling would be, if you press tab and the status bars are shown, they would be hidden, and if you press tab and the status bars are hidden, they would be shown.

Actual toggling would also be more useful than the current behavior, because it would let you see status bars for a while. For instance, that would be useful while selecting injured units for healing or retreat, or during a fight to target injured enemies. Selecting injured units in particular is difficult with the current behavior because you have to awkwardly hold down tab and shift at the same time while using the mouse.

Test Plan

Open any game, and verify that pressing tab shows status bars on all units you control (selected or not) even after releasing tab. Pressing tab again turns them off.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

causative created this revision.May 22 2017, 3:27 AM
causative updated this revision to Diff 2109.May 22 2017, 3:33 AM

remove stray else

elexis edited edge metadata.May 22 2017, 4:10 AM

Thanks for the update from #4534 and moving it to the XML file.

echotangoecho and hannibal had no real opinion on it, but I like it and borg- really like it. It was also discussed in D238 and might inspire a new aura hotkey that toggles the visualization.

There is one hypothetical issue though. recalculateStatusBarDisplaywas called onTick before if the key was pressed. Currently it's only called onSimulationUpdate and on updateGUIObjects if the selection changed.

So if the turn length is noticeably slow (500ms already is, if there is lag, there will be more), then a unit that moved into the screen doesn't have the health bar shown until the next turn.

This can be reproduced by starting an MP game, pressing F9, typing Engine.SetTurnLength(2000);.

So what could be done is moving these lines from updateGUIObjects

	if (g_ShowAllStatusBars)
		recalculateStatusBarDisplay();

to onTick. But then we must check how long this takes in case 200 units are selected, for example using let t = Engine.GetMicroseconds(); ... warn(Engine.GetMicroseconds() - t);. A turn should be processed in few milliseconds (like 50ms in the GUI would be ok ideally).

binaries/data/mods/public/gui/session/hotkeys/misc.xml
98 ↗(On Diff #2109)

remove whitespace in empty lines

causative added a comment.EditedMay 22 2017, 8:45 AM

So where exactly is misc.xml getting processed? Seems like the solution would be to process misc.xml once every tick, or immediately after a keyboard event, fixing responsiveness for several hotkeys. But perhaps that should be a different patch.

causative updated this revision to Diff 2114.May 22 2017, 9:21 AM

whitespace

Vulcan added a subscriber: Vulcan.May 22 2017, 4:02 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1302/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1303/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1304/ for more details.

causative updated this revision to Diff 2142.May 23 2017, 2:48 AM

Update the status bar display every 100 ms. Verified that this takes up to about 5 ms if there are a lot of women on the screen. The other option would have been to turn every status bar on when the user presses tab, even those that are off-screen, which would remove the need to update. I chose not to do this, because it seems the shapes might get clipped fairly late in the rendering process.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1319/ for more details.

causative updated this revision to Diff 2144.May 23 2017, 5:57 AM

used a constant for 100ms

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1321/ for more details.

elexis accepted this revision.May 26 2017, 3:35 AM

Checking whether or not entities are on the screen can still be done directly in the simulation instead of doing it in the GUI by calling into the simulation. recalculateStatusBarDisplay could then just pass all entities. This check could also be done every 100ms instead of onTick. But this doesn't have to be done in this diff. Also not sure if it's really nice to have an simulation independent timer in the simulation.

binaries/data/mods/public/gui/session/session.js
15 ↗(On Diff #2144)

missing semicolon

g_StatusBarUpdateRate at least caps for consts in the GUI have been phased out and made consistent (so that if we ever decide to change a const to a var, we don't have to replace all occurances, also http://trac.wildfiregames.com/wiki/Coding_Conventions say g_Foo for globals while not mentioning a special case for consts, also caps are ugly to read).

Don't make that the first global of the file as it's really unimportant. lastTickTime seems to be the most related one.

729 ↗(On Diff #2144)

I wonder how this doesn't throw warnings when doing mathmatical operations, since we save a Date object, not the number of milliseconds since the epoch

747 ↗(On Diff #2144)

Make that comment agnostic of the value. Actually the comment is unneeded.

748 ↗(On Diff #2144)

unneeded parenthesis

now % g_StatusBarUpdate <= tickLength removes the unneeded 0 and subtraction

This revision is now accepted and ready to land.May 26 2017, 3:35 AM
This revision was automatically updated to reflect the committed changes.