-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Implement skirmish game saves. #16419
Conversation
6c15e7c
to
ab4ec93
Compare
Updated - added saving and loading for skirmish observer viewports. |
ab4ec93
to
617ae8c
Compare
Needs rebase now. |
617ae8c
to
7eb35c8
Compare
Rebased. This should now be much less intimidating to review :) |
Had that "no loading progress" issue again. RA, map Maelstrom, 4 bots and me as spectator. |
Was the game saved on the current version of the PR, or reloaded from an earlier version? Can you please upload the orasav file? |
Pretty sure it was on this version, but not 100% sure.
|
That is very weird, the save file is missing data for frame 2, so the playback stalls indefinitely waiting for the orders that never arrive. The issue that triggers this must be inside |
Could this be caused by #16052? |
7eb35c8
to
81397dd
Compare
Good thinking, but not quite. Until the latest rebase this PR was based on bleed before that PR was merged. Your comment gave me the lead I needed to identify the real issue - which was exactly the bug that #16052 fixed! The problem was triggered by order packets that started with an immediate order, which caused the check in To quote myself from #16052: "Well derp, the old code was indeed obviously wrong." This was tricky to reproduce in the first place because it relied on non-deterministic timing, but i'm fairly confident that rebasing on latest bleed will have fixed this. |
Got this while trying to save a 6-AI skirmish. No savegame created.
|
Do you have the replay from that game? Edit: Ah nevermind, I bet this is because the target actor was already dead. |
81397dd
to
aa2039d
Compare
Fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This save point replay of a skirmish game on the latest revision of this PR crashes when it is almost loaded:
OpenRA engine version {DEV_VERSION}
Red Alert mod version {DEV_VERSION}
on map 3fcca6fe322b77e12ef3b965a44f6f71feac34fb (Mass Confliction by Nuke'm Bro.).
Date: 2019-04-27 21:19:48Z
Operating System: Linux (Unix 4.15.0.47)
Runtime Version: Mono 5.20.1.19 (tarball Thu Apr 11 09:02:17 UTC 2019) CLR 4.0.30319.42000
Exception of type `System.NullReferenceException`: Object reference not set to an instance of an object
at OpenRA.Mods.Common.Traits.BotModules.Squads.Squad..ctor (OpenRA.Traits.IBot bot, OpenRA.Mods.Common.Traits.SquadManagerBotModule squadManager, OpenRA.Mods.Common.Traits.BotModules.Squads.SquadType type, OpenRA.Actor target) [0x0001f] in <038e78e4d6734379add7e3e1f39950e2>:0
at OpenRA.Mods.Common.Traits.BotModules.Squads.Squad.Deserialize (OpenRA.Traits.IBot bot, OpenRA.Mods.Common.Traits.SquadManagerBotModule squadManager, OpenRA.MiniYaml yaml) [0x000ac] in <038e78e4d6734379add7e3e1f39950e2>:0
at OpenRA.Mods.Common.Traits.SquadManagerBotModule.OpenRA.Traits.IGameSaveTraitData.ResolveTraitData (OpenRA.Actor self, System.Collections.Generic.List`1[T] data) [0x0028b] in <038e78e4d6734379add7e3e1f39950e2>:0
at OpenRA.World.Tick () [0x00066] in <c159506b19684c1fa30cdf1328bff380>:0
at OpenRA.Game.InnerLogicTick (OpenRA.Network.OrderManager orderManager) [0x00201] in <c159506b19684c1fa30cdf1328bff380>:0
at OpenRA.Game.LogicTick () [0x0004a] in <c159506b19684c1fa30cdf1328bff380>:0
at OpenRA.Game.Loop () [0x000e0] in <c159506b19684c1fa30cdf1328bff380>:0
at OpenRA.Game.Run () [0x0003c] in <c159506b19684c1fa30cdf1328bff380>:0
at OpenRA.Game.InitializeAndRun (System.String[] args) [0x00010] in <c159506b19684c1fa30cdf1328bff380>:0
at OpenRA.Program.Main (System.String[] args) [0x00044] in <c159506b19684c1fa30cdf1328bff380>:0
aa2039d
to
69d6868
Compare
Crash was caused by trying to restore bot state during a replay, where bots are not active. Fixed by adding an explicit replay check. |
Is anything blocking review here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor thing (not a blocker if it's not an easy fix): When loading a replay via 'Watch' the loading screen still says 'Loading Saved Game', the load screen doesn't differentiate here.
Apart from that, looks good to me.
I'm not sure what we should be showing in that case. "Loading replay" isn't really more or less correct - we are replaying the loading of the save game. |
OK, let's just leave it like that then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't find any issue here, looks all good to me.
This PR enables save games for skirmishes after implementing
IGameSaveTraitData
on the bot modules.Depends on #16411.