FAForever / fa

Lua code for FAF
221 stars 228 forks source link

Fix unit tooltip docking for non-startup layouts #6216

Closed PaletzTheWise closed 1 month ago

PaletzTheWise commented 1 month ago

Description of the proposed changes

Issue

Non-startup layouts (when you change the layout with alt-up or alt-down) don't dock unit tooltip correctly, here the tooltip has sunk beneath the construction control:

DockingGlitch

Root cause

Unlike other controls, unitview.lua does not fetch the current layout every time it uses it. Instead, https://github.com/FAForever/fa/commit/241c13cf4198d6aa81f70251eae615627a934451 introduced an optimization to only load the layout on startup into a variable and then call its methods through the variable. The optimization was motivated by the fact that unitview calls the layout methods in an OnFrame function and fetching it on each frame would be expensive.

Fix:

The main fix is to update the layout variable in SetLayout(). In addition to that:

Testing done on the proposed changes

Additional context

There is still this glitch in the left layout when you switch to it (regardless of whether it was the startup layout), but it is a pre-existing issue: Non-startup-left-production-glitch

Checklist

lL1l1 commented 1 month ago

Changes are documented in the changelog for the next game version What's the process around this? Make a draft PR and add a snippet to changelog/snippets based on the PR number and the nature of the change?

Yes you open this PR as a draft and then add the snippet to changelog/snippets. So it would be like fix.6216.md with text:

- (#6216) Fix unit view window not changing layout when the UI layout is changed.
PaletzTheWise commented 1 month ago

I have added the changelog snippet but with my own description. I have read lL1l1's response in my email client and somehow missed the proposed text. Well, let me know if you find my description inferior.

@clyfordv, since you have reviewed my previous change in the same area, perhaps you could review this too?