Opened 3 years ago

Closed 16 months ago

Last modified 16 months ago

#2807 closed defect (fixed)

[PATCH] Check the list of enabled mods when running a replay from the command line

Reported by: Itms Owned by: Arno Hemelhof
Priority: Should Have Milestone: Alpha 21
Component: Core engine Keywords: patch
Cc: Patch:

Description (last modified by Itms)

Right now the replay mode fails when no mod is specified, we should check that and inform the user about what is happening.

Maybe this is part of a more general task, which is including mod data in commands.txt and other logging stuff.

We can use the list of mods in the commands.txt to make sure everything is fine.

Attachments (5)

replayWithoutModsErrorMessage_v1.diff (1.3 KB) - added by Arno Hemelhof 17 months ago.
Displays an error message when you try to view a replay without specifying any mods
replayWithoutModsErrorMessage_v2.diff (1.3 KB) - added by Arno Hemelhof 17 months ago.
Changed based on the remarks of comment 7
replayWarnOnModMismatch_v1.diff (2.6 KB) - added by Arno Hemelhof 16 months ago.
replayWarnOnModMismatch_v2.diff (2.6 KB) - added by Arno Hemelhof 16 months ago.
replayWarnOnModMismatch_v2_2.diff (2.6 KB) - added by Arno Hemelhof 16 months ago.
A new version which resolves a file confict (to make applying and reviewing easier)

Download all attachments as: .zip

Change History (20)

comment:1 Changed 3 years ago by mimo

It works for me on linux.

comment:2 Changed 3 years ago by Itms

Summary: Replay mode broken againReplay mode unable to load maps on Windows

Apparently it's the VFS not finding the file, maybe related to mod selection changes?

I update the ticket description.

comment:3 Changed 3 years ago by Itms

Cc: leper added

comment:4 Changed 3 years ago by Itms

Description: modified (diff)
Keywords: simple added
Milestone: Alpha 17Backlog
Priority: Release BlockerShould Have
Summary: Replay mode unable to load maps on WindowsDisplay a nice error message when trying to run a replay without mods

Um, I just understood that now the replay mode needs to know which mod(s) to load. I'm updating the docs and this ticket.

comment:5 Changed 18 months ago by elexis

Trivial, just show the user a hint if g_args has "replay" but no "mod" argument. File: main.cpp

Changed 17 months ago by Arno Hemelhof

Displays an error message when you try to view a replay without specifying any mods

comment:6 Changed 17 months ago by Arno Hemelhof

Keywords: review patch added
Milestone: BacklogAlpha 21
Summary: Display a nice error message when trying to run a replay without mods[PATCH]Display a nice error message when trying to run a replay without mods

This patch just implements an error message. Should something else be done when nothing is specified, like loading public?

comment:7 Changed 17 months ago by elexis

Cc: leper removed
Keywords: review removed
Summary: [PATCH]Display a nice error message when trying to run a replay without mods[PATCH]Display a nice error message when trying to run a nonvisual replay without mods
  • Looks like it works, but in fact its broken as the XML library needs to be deinitialized on shutdown, see that CXeromyces::Terminate(); call below.
  • should use LOGERROR (without newline at the end)
  • "None were detected" doesn't add any information that isn't contained in the previous sentence.
  • The message is clearly directed at users who are not familiar with the code or technical background. So the term "official mod" is a bit confusing / counterintuitive and should be avoided. I recall seeing "Did you mean this?" being printed out in some tool.
At least one mod should be specified! None were detected!\nIf you intended to use the official mod, do so by adding the argument '-mod=public'
  • Maybe it would be better to first load the replay, extract the mods that specific replay wants, attempt to load them and then throw an error message if the specified mods were not found. Trying to replay something with a different set of mods doesn't seem to make any sense and if some developer wants to hack something with mods, he can just hack the replay file. This would be true for the visual replay as well.

Changed 17 months ago by Arno Hemelhof

Changed based on the remarks of comment 7

comment:8 Changed 17 months ago by Arno Hemelhof

Keywords: review added
Owner: set to Arno Hemelhof
Status: newassigned
Summary: [PATCH]Display a nice error message when trying to run a nonvisual replay without mods[PATCH] Display a nice error message when trying to run a nonvisual replay without mods

Thank's for the review. I based myself on the way it is handled when the replay file isn't found (Line 443) and CXeromyces::Terminate() isn't being called there. I have added the terminate call, used LOGERROR and rephrased my error message.

comment:9 Changed 17 months ago by Itms

Description: modified (diff)
Keywords: review removed
Summary: [PATCH] Display a nice error message when trying to run a nonvisual replay without mods[PATCH] Check the list of enabled mods when running a replay from the command line

Hi, and thanks for your work so far. :)

When this ticket was created, we didn't have the list of enabled mods included in the commands.txt file. Now that we have that, it should be possible to check that the list of enabled mods is the same as in the commands file.

Currently the check is done in the visual replay menu but not when using the command line. I think a mismatch in the mod lists should trigger a warning instead of an error, so people using the command line are still allowed to do what they want.

This would be a more complex task. Would you be up for it? You can come to our IRC channel if you need more information.

Also, for next time, please take a look at our new way of SubmittingPatches: we're making changes to our review process.

Changed 16 months ago by Arno Hemelhof

comment:10 Changed 16 months ago by Arno Hemelhof

Keywords: rfc added

I have written a new patch. The plan was to automatically load the mods specified in the mod list of the replay file, but to parse the JSON, I needed a JSContext*.The context of the Simulation2 instance however, is created in the constructor of CGame which requires the mods to already be loaded. To change the order in which these action occur, a rewrite would be needed. Since this is my first ticket, I don't think I have enough experience to do such a rewrite. A new ticket could be made for this feature if necessary.

This patch prints out an error message when the replay is trying to be loaded without any mods specified. It also displays a warning which shows which mods are missing based on the mod list in the replay file,and which mods were loaded which aren't in the replay file. Would it be better to let the program continue when no mods are loaded so it can print out which mods are missing? The program would still crash in that case.

comment:11 Changed 16 months ago by elexis

Keywords: review added; simple rfc removed

Sounds right, I'll take a look.

comment:12 Changed 16 months ago by elexis

Keywords: rfc added; review removed

One more thing though. There is this mock mod "user" which is launched in the release version but not in the svn working copy. See the logic of hasSameMods in functions_utility_loadsave.js. The mod should just be ignored, it is not relevant in this context.

Changed 16 months ago by Arno Hemelhof

comment:13 Changed 16 months ago by Arno Hemelhof

Now it ignores the mod "user" in both loops.

Changed 16 months ago by Arno Hemelhof

A new version which resolves a file confict (to make applying and reviewing easier)

comment:14 Changed 16 months ago by elexis

Resolution: fixed
Status: assignedclosed

In 18574:

Print warnings when no or the wrong list of mods is passed when starting non-visual replays. Patch by ArnH, fixes #2807.

comment:15 Changed 16 months ago by elexis

Keywords: rfc removed

Thanks for the patch! I have removed the whitespace in emptylines and unneeded comments.

Note: See TracTickets for help on using tickets.