这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@PunkPun
Copy link
Member

@PunkPun PunkPun commented Sep 14, 2024

Fixes #21290

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

@RoosterDragon RoosterDragon left a comment

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.

Copy link
Member

@Mailaender Mailaender left a comment

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
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