Skip to content

Fix opening up skirmish menu making it impossible to leave map editor#21580

Merged
Mailaender merged 1 commit into
OpenRA:bleedfrom
PunkPun:map-editor
Sep 15, 2024
Merged

Fix opening up skirmish menu making it impossible to leave map editor#21580
Mailaender merged 1 commit into
OpenRA:bleedfrom
PunkPun:map-editor

Conversation

@PunkPun

@PunkPun PunkPun commented Sep 14, 2024

Copy link
Copy Markdown
Member

Fixes #21290

@PunkPun PunkPun added this to the Next Release milestone Sep 14, 2024

@RoosterDragon RoosterDragon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hack makes sense to me, and I believe it's acceptable.

Background: In Game.RunAfterDelay the Game.IsCurrentWorld is there to protect against something having happened in the meantime. Because there was a delay, it's possible something else swapped out the world. If that happened we don't want to close - our goal was to close the world that was active at the time the OnQuit was called, not to close a different world that was made active whilst we were waiting for the delay to finish.

The if (!Game.IsCurrentWorld(world)) is there as the comment suggests - when you open the Skirmish menu it creates a new OrderManager - which means IsCurrentWorld now becomes false. If you close the Skirmish menu and then try and quit - they damage is done, we've already decided it's not the right world any more.

But that's just a side effect of our irredemably cursed top-level shared state in the Game class. Really the fact it's trying to keep track of a single order manager when there might be multiple in play (in this case, one running the editor and another for the skirmish menu should you start the game) that is quite broken here.

Given we're not inside a Game.RunAfterDelay - it's quite reasonable to expect that OnQuit has just been clicked and therefore we're trying to close the "correct" world. Since there's no delay there's no chance something could've snuck a wrong world under our noses that we close by mistake.

Therefore I'm happy with this approach, the approach is safe and closing the correct world. The "hack" is really that Game.cs is cursed, rather than anything with this specific approach being a danger. To remove the "hack" we need to fix our top-level plumbing which is a mission for another day.

@Mailaender Mailaender left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not seem to break anything.

@Mailaender Mailaender merged commit a9e5744 into OpenRA:bleed Sep 15, 2024
@Mailaender

Copy link
Copy Markdown
Member

Changelog

@PunkPun PunkPun deleted the map-editor branch September 15, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Game partially freezes when in map editor you press the play button, and then exit map editor

3 participants