Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove hacks around checking sync while disposing the shellmap #19714

Merged
merged 3 commits into from Oct 9, 2021

Conversation

abcdefg30
Copy link
Member

@abcdefg30 abcdefg30 commented Oct 3, 2021

#19657 made me investigate why those hacks/workarounds are needed. It turns out the sync of the world changes because all actors and effects are dropped from the (shellmap) world when it is being disposed this tick.
This PR comes with three commits:

  • We bail early when checking around unsynced code is not enabled, saving us from running the function inside try catch and making the code easier to read imo.
  • We no longer trigger an exception when the world is disposing (and thus the sync hash changed). I don't think it makes sense to have a check for sync changes when the world is about to get thrown out anyway.
  • We can now remove the HACKs and directly load into the editor.

@pchote pchote added this to the Next release milestone Oct 3, 2021
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Nice detective work!

Not tested yet, but changes look sensible.

OpenRA.Game/Sync.cs Show resolved Hide resolved
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

LGTM and now also tested.

@teinarss teinarss merged commit 4269fc6 into OpenRA:bleed Oct 9, 2021
@abcdefg30 abcdefg30 deleted the adjustRunUnsynced branch October 9, 2021 19:17
@anvilvapre
Copy link
Contributor

After disabled the Sync checkboxes in the settings dialog bleed crashes once I deploy my MVC in a Skirmish game. I suspect related to this commit? If i checkout the commit before - no crash.

Exception of type `System.InvalidOperationException`: The current order generator may not be changed from synced code
   at OpenRA.Sync.AssertUnsynced(String message) in /home/taol/source/cs/OpenRA/OpenRA.Game/Sync.cs:line 208
   at OpenRA.World.set_OrderGenerator(IOrderGenerator value) in /home/taol/source/cs/OpenRA/OpenRA.Game/World.cs:line 153
   at OpenRA.Mods.Common.Widgets.WorldInteractionControllerWidget.HandleMouseInput(MouseInput mi) in /home/taol/source/cs/OpenRA/OpenRA.Mods.Common/Widgets/WorldInteractionControllerWidget.cs:line 181
   at OpenRA.Widgets.Ui.HandleInput(MouseInput mi) in /home/taol/source/cs/OpenRA/OpenRA.Game/Widgets/Widget.cs:line 125
   at OpenRA.DefaultInputHandler.<>c__DisplayClass5_0.<OnMouseInput>b__0() in /home/taol/source/cs/OpenRA/OpenRA.Game/Input/InputHandler.cs:line 50
   at OpenRA.DefaultInputHandler.OnMouseInput(MouseInput input) in /home/taol/source/cs/OpenRA/OpenRA.Game/Input/InputHandler.cs:line 51
   at OpenRA.Platforms.Default.Sdl2Input.PumpInput(Sdl2PlatformWindow device, IInputHandler inputHandler, Nullable`1 lockedMousePosition) in /home/taol/source/cs/OpenRA/OpenRA.Platforms.Default/Sdl2Input.cs:line 149
   at OpenRA.Platforms.Default.Sdl2PlatformWindow.PumpInput(IInputHandler inputHandler) in /home/taol/source/cs/OpenRA/OpenRA.Platforms.Default/Sdl2PlatformWindow.cs:line 459
   at OpenRA.Renderer.EndFrame(IInputHandler inputHandler) in /home/taol/source/cs/OpenRA/OpenRA.Game/Renderer.cs:line 312
   at OpenRA.Game.RenderTick() in /home/taol/source/cs/OpenRA/OpenRA.Game/Game.cs:line 728
   at OpenRA.Game.Loop() in /home/taol/source/cs/OpenRA/OpenRA.Game/Game.cs:line 834
   at OpenRA.Game.Run() in /home/taol/source/cs/OpenRA/OpenRA.Game/Game.cs:line 866
   at OpenRA.Game.InitializeAndRun(String[] args) in /home/taol/source/cs/OpenRA/OpenRA.Game/Game.cs:line 295
   at OpenRA.Launcher.Program.Main(String[] args) in /home/taol/source/cs/OpenRA/OpenRA.Launcher/Program.cs:line 32

@anvilvapre
Copy link
Contributor

If you'd like to keep the assertion, perhaps you will want to called RunUnsynched but with the first argument checkSync always to false.

@abcdefg30
Copy link
Member Author

See #19735. I filed #19741 to fix it.

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.

None yet

4 participants