Added a warning before loading incompatible replays

[Mailaender]

This was demanded at #4528 (comment) by @pchote and is also useful to reduce the number of support requests https://github.com/OpenRA/OpenRA/wiki/FAQ#old-replays-crash-the-game.

[obrakmann]

These strings need a line break somewhere, they are too wide for the dialog box.

[obrakmann]

Personally, I wouldn't allow watching the replay anyway despite it being incompatible. Just show the user a window that the replay can't be played. Everything else is bogus (especially the last case with the missing map), and it allows you to avoid a whole lot of special cases. It's probably also better not to not separate the tests for mod and version.

Edit: To elaborate, I think it should look something like this:

            var mod = replayMeta.GameInfo.Mod;
            var version = replayMeta.GameInfo.Version;

            var allMods = ModMetadata.AllMods;

            if (allMods.ContainsKey(mod) && allMods[mod].Version == version)
                return true;
            else if (!allMods.ContainsKey(mod))
                ConfirmationDialogs.CancelPrompt("Incompatible replay", "This replay was recorded with an unknown mod:\n{0}.".F(replayMeta.GameInfo.Mod), onCancel);
            else if (!allMods[mod].Version != version)
                ConfirmationDialogs.PromptConfirmAction("Incompatible replay", "This replay was recorded with an incompatible version:\n{0}.".F(replayMeta.GameInfo.Mod), onConfirm, onCancel);
            else if (replayMeta.GameInfo.MapPreview.Status != MapStatus.Available)
                ConfirmationDialogs.CancelPrompt("Incompatible replay", "This replay was recorded with an unavailable map:\n{0}.".F(replayMeta.GameInfo.MapUid), onCancel);

            return false;

End edit

If both mod and version don't match, the game isn't able to switch mods when the user clicks "Confirm" to watch the replay anyway and the user is dropped off at the main menu without explanation. This will probably the case users would encounter most often when they open .orareps from somewhere on the web, and could be avoided

There's also a minor issue with replays that don't have the proper metadata information in their header. It causes the text to look cutoff.

[phrohdoh]

I agree with what @obrakmann said with the addition of stating the replay's mod's version to indicate the correct version of OpenRA that can view the replay.

[DanAE111]

Change confirm to continue?

Might not hurt to add that continuing can result in abnormalities, desyncs, and crashes.

So all up something like this:

Incompatible Replay

This replay was recorded with engine version
Playback is not guaranteed to be error free.

Continue Cancel

[pchote]

The dialog should also be displayed when the check in 2cdcd0f is triggered, or if this pr supersedes it then that should be reverted. That fix should have been sufficient to fix @obrakmann's issues, but I don't think he posted any stack traces to debug against so I don't know what is going wrong here.

[pchote]

The reason why I didn't take these changes when I redid the mod switching stuff is that this metadata is already encoded in the replay, and verified by the game in the handshake and start game orders. I would prefer it if we could fix any remaining issues and hook up the dialogs for these instead of adding redundant data.

Actually, scratch that. Having these in the metadata block will make it easier to hook up file explorer / shell integration.

[Mailaender]

Updated.

You now can't start incompatible replays only cancel. Continue anyway made only sense when there was no replay meta block, but the replay is still compatible which is just useful now during development of this pull request, but not afterwards.

The game will now also automatically switch to the correct mod at startup which is required for #4528.

[Mailaender]

Also cleaned the code a bit to avoid redundancy.

[penev92]

Doesn't IsReplayCompatible() sound better?

[Mailaender]

Also thought about that. It does not only check that though, but also displays popup windows.

[chrisforbes]

We should probably split this so it does one thing.

[Mailaender]

Already moved the UI fluff to IncompatibleReplayDialog during the last set of requested changes.

[phrohdoh]

I'd like to request the namechange that @penev92 mentioned, Check is just doing something while the Is prefix is pretty universal for bools.

[obrakmann]

I tested this together with #4528, and I'm happy with it now. The game did the expected thing in all situations I tested (wrong version, missing mod, missing map).

The only minor nit I have is that the cancel button isn't centered in ra (and d2k/ts, presumably). I suggest just adding a new dialog definition to chrome/dialogs.yaml. It's just copy&pasting a few lines of yaml.

👍 with the fixed dialog.

[Mailaender]

Added the CANCEL_PROMPT as requested.

[penev92]

Testing with relpays not-from-this-PR leads to:

(19:14:39) penev: I get "unknown" mod for every single one of those
I try to run a replay (older one) and end up at https://github.com/OpenRA/OpenRA/pull/7353/files#diff-a56cfc96cff0843d1d5b19bce9830e5aR27

I'd suggest switching the mod and version checks to avoid the WTF? they cause in that case.

[Mailaender]

That is on purpose. Replays from old releases (different version) and from very old releases (not even a version stamp in the replay) will be incompatible.

[penev92]

It will be a while before there is a difference between those two.
But either way getting an "unknown mod" message when selecting a replay you recorded yesterday in RA is just wrong.

[Mailaender]

I was already asked to remove the continue anyway button, because it is only useful on the development branch during a brief period.

[penev92]

No, I get that the version is incompatible. But the game tells me the mod is unknown. That is my problem with it.

[pchote]

I hit the following crash when trying to load an old replay file:

Exception of type `System.NullReferenceException`: Object reference not set to an instance of an object
  at OpenRA.Mods.Common.LoadScreens.BlankLoadScreen.StartGame (OpenRA.Arguments args) [0x0019d] in /home/paul/OpenRA/OpenRA.Mods.Common/LoadScreens/BlankLoadScreen.cs:84 
  at OpenRA.Game.InitializeMod (System.String mod, OpenRA.Arguments args) [0x003d7] in /home/paul/OpenRA/OpenRA.Game/Game.cs:372 
  at OpenRA.Game.Initialize (OpenRA.Arguments args) [0x002d0] in /home/paul/OpenRA/OpenRA.Game/Game.cs:264 
  at OpenRA.Program.Run (System.String[] args) [0x00007] in /home/paul/OpenRA/OpenRA.Game/Support/Program.cs:109 
  at OpenRA.Program.Main (System.String[] args) [0x00050] in /home/paul/OpenRA/OpenRA.Game/Support/Program.cs:39 

[pchote]

Moving the CheckReplayCompatibility check immediately after this (and adding a null check inside it) should fix my crash and polish issue that @penev92 explained.

[pchote]

You will also need to reorder the checks inside CheckReplayCompatibility to first check for replayMeta == null, then version == null, then mod == null.

[Mailaender]

Fixed.

[pchote]

The dialog type is incorrect under TD, and probably also the other mods:

This is an informational prompt, and so the button should say "Ok" or "Continue" and be positioned at the right of the dialog. Cancel and the left-hand positioning implies that the game should quit or otherwise backtrack.

[Mailaender]

Cancel and the left-hand positioning implies that the game should quit or otherwise backtrack.

Which is exactly what it does. When you select an incompatible replay you will be brought back to the replay browser.

[pchote]

That isn't what happens when you launch from the OS file browser or commandline.

[Mailaender]

I had a [Continue] button at first, but it was meant to load the replay anyway despite warnings which was removed on request.

[pchote]

Needs a rebase.

[Mailaender]

Rebased.

[pchote]

This will still throw when replayMeta is null. You can fall back to the current mod if it is.

[Mailaender]

CheckReplayCompatibility will catch that so it can never throw.

[pchote]

This code still runs and throws after CheckReplayCompatibility displays its error dialog.

[Mailaender]

Woops, fixed.

[phrohdoh]

This is pretty nit-picky of me but since you are really hardcoding the different reasons here this string may as well include the "an" that all of the reasons start with, instead of duplicating it.

[phrohdoh]

After the two comments I made are addressed: 👍.

[Mailaender]

Tried to please all the nit-pickers.

[penev92]

Thank you

Changelog