Return-To-The-Roots / s25client

Return To The Roots (Settlers II(R) Clone)
http://www.rttr.info
GNU General Public License v2.0
470 stars 75 forks source link

Refactor PersistentWindowSettings and persist window minimized state #1609

Closed falbrechtskirchinger closed 12 months ago

falbrechtskirchinger commented 1 year ago
Flamefire commented 1 year ago

Any chance you can come up with a test especially for the minimized state restore? Should be enough to use a dummy window with a CGI ID matching an "enabled" window

Just want to be sure we'd catch e.g. the virtual call bug in the ctor you found.

falbrechtskirchinger commented 1 year ago

Any chance you can come up with a test especially for the minimized state restore? Should be enough to use a dummy window with a CGI ID matching an "enabled" window

Done. Let me know if this is what you had in mind.

Just want to be sure we'd catch e.g. the virtual call bug in the ctor you found.

That's rather difficult to test. I couldn't call SetMinimized() from the constructor, because that ultimately calls Resize(), and clang-tidy would rightfully warn, that this doesn't call the most derived Resize() as one might expect/want. Instead, I'm calling Window::Resize() myself, which is already called from the constructor, so I wouldn't expect any unanticipated side effects.

falbrechtskirchinger commented 1 year ago

@Flamefire Rebased and resolved the one-line merge conflict in IngameWindow.h. You can re-enable auto-merge.