HomeWildfire Games

Rewrite developer overlay to use class syntax, one class per checkbox, a class…
AuditedrP23081

Description

Rewrite developer overlay to use class syntax, one class per checkbox, a class for the EntityState overlay and TimeWarp debug feature, refs #5387.

Using 22 classes instead of 1 class (refs rP22370/D1928) leverages more benefit of the paradigm.
In particular it means the checkboxes can own the EntityState and TimeWarp helpers and the DeveloperOverlay itself can remain independent.
Improve performance by 200 microseconds per turn by unsubscribing from onSimulationUpdate when the developer overlay is not opened, refs rP23076 / D2378.
Move TimeWarp from input.js from rP8803 to independent class using hotkey release event from rP19028, refs #3194.

Differential Revision: https://code.wildfiregames.com/D2383

Event Timeline

Freagarach raised a concern with this commit.Oct 26 2019, 10:35 AM
Freagarach added a subscriber: Freagarach.

canControlAll does not function as intended, see respective inline.

/ps/trunk/binaries/data/mods/public/gui/session/developer_overlay/DeveloperOverlayCheckboxes.js
29

playerState && playerState.controlsAll?

/ps/trunk/binaries/data/mods/public/gui/session/input.js
285–286

This checks whether the selected entitie's player can control all. Which means that despite the player canControlAll they cannot control an enemy unit if the enemy player cannot control all.
Fix: g_ViewedPlayer instead of entState.player.

This commit now has outstanding concerns.Oct 26 2019, 10:35 AM
elexis added inline comments.Oct 26 2019, 12:30 PM
/ps/trunk/binaries/data/mods/public/gui/session/developer_overlay/DeveloperOverlayCheckboxes.js
29

The result of the function will be used to assign a checked property of a Checkbox object, that requires the value to have the correct type, see rP22538 (and for observers not viewing a playerperspective you get undefined).

/ps/trunk/binaries/data/mods/public/gui/session/input.js
285–286

Oh, you're right, thanks!

Also as of this commit playerState.controlsAll is checked once per entity per function call instead of once per function call.

(In case controlAll was enabled one can skip testing if every entity is owned. On the other hand that is never the case, so in the average case where the player has a purely owned selection, testing for all entities being owned or controlAll being enabled will return one evaluation sooner than testing whether controlAll is enabled or all entities being owned.)

elexis requested verification of this commit.Oct 27 2019, 3:16 PM

Thanks for the report!

This commit now requires verification by auditors.Oct 27 2019, 3:16 PM
Freagarach accepted this commit.Oct 27 2019, 9:02 PM

Concern fixed by rP23098 :D

All concerns with this commit have now been addressed.Oct 27 2019, 9:02 PM