HomeWildfire Games

Implement hotkeys and mousescroll to change tabs in tabbed pages (credits…


Implement hotkeys and mousescroll to change tabs in tabbed pages (credits, summary and options).
Save the latest page and graph in the summary screen
Implement a hotkey to open and close the summary screen for the replaymenu and ingame.

Patch By: ffffffff
Comments By: elexis
Differential Revision: https://code.wildfiregames.com/D810


bbDec 11 2017, 11:16 PM
Differential Revision
D810: Hotkey summary, hotkey tabs and panel/charts remember
rP20643: Fix missing i's in rP20079
Build Status
Buildable 4095
Build 7196: Post-Commit BuildJenkins

Event Timeline

bb added inline comments.Dec 27 2017, 11:21 PM

these are the wrong way around, followup commits didn't notice and left it like this

elexis added a subscriber: elexis.Jan 7 2018, 12:48 AM
elexis added inline comments.

...because the properties are neither gui nor sim data

elexis added inline comments.Jan 7 2018, 12:49 AM

(and then others don't sort either D1058)

elexis added inline comments.Jan 7 2018, 2:54 AM

Missing *
The order of functions alreadyi s completely random in this file anyhow, so not really a regression, but functions should be sorted, first input, then processing, then output (https://en.wikipedia.org/wiki/IPO_model) and the shorter functions first.


So it's an empty string if undefined and an object with two properties otherwise?


should use tabs instead of spaces


The reader might want to lookup the code if it's needed to distinguish empty string from undefined

elexis added inline comments.Jan 7 2018, 2:57 AM

Why isn't g_SelectedPanel grouped with g_SelectedChart as it is below?
Why are there two variables here while having an object with two properties elsewhere?

bb marked 4 inline comments as done.Jan 7 2018, 2:45 PM

rP20785 should fix the style issues


yet the * is missing


there lies a deeper duplication issue here: the way tabs in the summary screen are handled is fundamentally different from what we do in the credits/options and as I didn't want to mess with that in rP20684, it isn't nuked there. A proper fix should nuke the duplication and then the global would move to another file and become an index instead of a string, thus fixing both issues.

elexis added inline comments.Sep 18 2019, 12:31 AM

Dialog toggle hotkeys should be used for opening or closing dialogs, but not for switching the current page away (as dialog actions are reversible, but page switches are not), so IMO the page switch changes are better off with a separate hotkey. For example we also dont want someone to close the gamesetup page after having setup the settings (for sometimes half an hour) and then pressing escape (but with shift+esape it would be ok, similar to delete without confirmation hotkey).

elexis added inline comments.Sep 18 2019, 12:47 AM

For the replay menu, it is also a "recoverable" action. Mostly the page should be opened as a dialog, but the dialog looks ugly, as those outer 20-ish margin pixels are mostly black, while ingame they show the map and units.
But closing the patch after having finished a game with the close hotkeys still seems wrong to me, it should be consistent for all pages.
So perhaps this can be improved:

elexis added inline comments.Oct 16 2019, 2:55 PM

explicitResume is meant to be a boolean, undefined if you will, but not 0.
If the return value is not defined, undefined, or otherwise falsy, it doesnt have to be passed to begin with.

The 0 return value from PopGuiPageCB seems to have just been used as a placeholder because anything must be returned in the code prior to rP22676 if the page was opened with a callback argument IIRC.

The 0 seems to oringate at rP17422 but I think that was copypasta too.