Page MenuHomeWildfire Games

Fix line misaligment in gamesetup settings.
Needs ReviewPublic

Authored by nani on Jan 20 2019, 6:57 PM.

Details

Reviewers
elexis
bb
smiley
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Chat and settings GUI are only updated when distributeSettings is called but distributeSettings only is called when the windows resizes. Call it once at init so resizes to the correct size at the beginning.

Test Plan

Chat and panels looking aligned.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

nani created this revision.Jan 20 2019, 6:57 PM
Stan added reviewers: Restricted Owners Package, elexis, bb.Jan 20 2019, 6:58 PM

Was it really a wrong size and not just one of the panels being visible when it should be hidden? Which GUI object is that?

smiley requested changes to this revision.EditedJan 21 2019, 8:50 AM
smiley added a subscriber: smiley.

Why this does not led to the issue before the commit that has the concern raised?
Previously, the sizes of the two panel would be adjusted onTick in updateSettingsPanelPosition.

let settingsBackground = Engine.GetGUIObjectByName("settingsBackground");
let backgroundSize = settingsBackground.size;
backgroundSize.left = settingsPanelSize.left;
settingsBackground.size = backgroundSize;

But now, if the offset is 0, that code would not be reached.

Therefore, IMO, this is a workaround. But if someone thinks otherwise, feel free to remove the status.

This revision now requires changes to proceed.Jan 21 2019, 8:50 AM

Adding a single function call to update more than strictily necessary in the GUI is a common simplification to avoid complex logic checking which things have to be updated.
Should still check whether this is the best place to add the line and the smallest code tree to call necessary.
For example initGUIObjects would be less unspecific than init(. Many people wanted to and have added many things in int, perhaps we should add a comment that people should stop addng things there.)
initSettingsTabButtons() would even be more specific than initGUIObjects.
Perhaps instead of distributeSettings, one could call updateSettingsPanelPosition() (IIRC nani already asked about 0 vs. undefined, now I know why. The answer to that can be found in the JS documentation).

nani updated this revision to Diff 7399.Sat, Jan 26, 11:17 PM

Seems the real culprit was in the xml file. Some value was greater than expected making that the initial condition always return 0.

smiley added inline comments.Sun, Jan 27, 5:02 AM
binaries/data/mods/public/gui/gamesetup/gamesetup.xml
146

Offset by 5 like the others?

nani updated this revision to Diff 7401.Sun, Jan 27, 1:02 PM

Add the offset of 5.

Does look correct now apart from this one thing.
(Would accept the revision but that would be pretty meaningless.)

binaries/data/mods/public/gui/gamesetup/gamesetup.xml
146

Actually, the above comment is wrong. I wrongly compared x1 with y2. As you can see, x2 does not have any offset either. And also from the looks of the thing it seems like it's only needed for y.

nani updated this revision to Diff 7405.Sun, Jan 27, 7:16 PM

Back to how it was.

From the original diff I gather that the size of this object is computed by JS? (Then it could be cleaner to not duplicate that information and remove the size from XML altogether and compute it once upon start. Then there aren't two places in the code that have to be kep in sync with people having to do math and comparisons.)