Developer tools disabled checkboxes are now filtered out during instantiation of DeveloperOverlay
Unit Tests Skipped
First off, thank you for the patch! Don't hesitate to drop by on IRC if you have any questions.
As noted in the patch it would be good to only hide the checkbox, not the entire line.
Please keep alphabetic order (nick if present) :)
|12 ↗||(On Diff #11922)|
We prefer let over var whenever possible.
|17–21 ↗||(On Diff #11922)|
Can be combined.
Perhaps this could be useful for other functions as well?
- Engine.GetPlayerID() can change during the course of the game using the developer overlay "change player" option.
- Other hypothetical enabled properties could change arbitrarily.
Therefore all checkboxes must be initialized in the beginning and later when something changes, the checkbox should change accordingly.
So to be more precise:
DeveloperOverlayCheckboxes.prototype.ControlAll.enabled changes if and only if Engine.GetPlayerID() changes, so whenever Engine.GetPlayerID() changes, the enabled part should be refreshed ideally.
This could be done using something like this.registerViewedPlayerChangeHandler(this.rebuild.bind(this)); or by using the patch from #5670.
could you please reuse variable above enabled to assign into hidden?
With this change, Checkbox is missing but text label is still there.
So it should be or only grayed out so set to enabled = false, or if hidden, text label should be hidden as well and in that case, there should not be blank space visible.
Also Promote selected units should be disabled when player is observer in regards why Control all units is disabled in that case.