Opened 10 years ago
Last modified 8 years ago
#2807 closed defect
[PATCH] Check the list of enabled mods when running a replay from the command line — at Version 9
Reported by: | Itms | Owned by: | Arno Hemelhof |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 21 |
Component: | Core engine | Keywords: | patch |
Cc: | Patch: |
Description (last modified by )
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.
Change History (12)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
Summary: | Replay mode broken again → Replay 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 by , 10 years ago
Cc: | added |
---|
comment:4 by , 10 years ago
Description: | modified (diff) |
---|---|
Keywords: | simple added |
Milestone: | Alpha 17 → Backlog |
Priority: | Release Blocker → Should Have |
Summary: | Replay mode unable to load maps on Windows → Display 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 by , 8 years ago
Trivial, just show the user a hint if g_args has "replay" but no "mod" argument. File: main.cpp
by , 8 years ago
Attachment: | replayWithoutModsErrorMessage_v1.diff added |
---|
Displays an error message when you try to view a replay without specifying any mods
comment:6 by , 8 years ago
Keywords: | review patch added |
---|---|
Milestone: | Backlog → Alpha 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 by , 8 years ago
Cc: | 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.
by , 8 years ago
Attachment: | replayWithoutModsErrorMessage_v2.diff added |
---|
Changed based on the remarks of comment 7
comment:8 by , 8 years ago
Keywords: | review added |
---|---|
Owner: | set to |
Status: | new → assigned |
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 by , 8 years ago
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.
by , 8 years ago
Attachment: | replayWarnOnModMismatch_v1.diff added |
---|
It works for me on linux.