-
Notifications
You must be signed in to change notification settings - Fork 84
[ZH] Simulate Replays with CSV files #925
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
base: main
Are you sure you want to change the base?
[ZH] Simulate Replays with CSV files #925
Conversation
…s::appendNextCommand()
# Conflicts: # GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp # GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
# Conflicts: # GeneralsMD/Code/GameEngine/Source/Common/CommandLine.cpp
# Conflicts: # GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp # GeneralsMD/Code/Main/WinMain.cpp
# Conflicts: # GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp
…defined (even in Release mode)
# Conflicts: # GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp
This reverts commit 6b23bf8.
This reverts commit a1266af.
…Playback(). This was causing the last gametime to be incorrect in console output
# Conflicts: # GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h
# Conflicts: # GeneralsMD/Code/GameEngine/CMakeLists.txt # GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h # GeneralsMD/Code/GameEngine/Source/Common/CommandLine.cpp # GeneralsMD/Code/GameEngine/Source/Common/GameMain.cpp # GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp
…yList # Conflicts: # GeneralsMD/Code/GameEngine/Source/Common/CommandLine.cpp
This reverts commit 0f9a0cd.
Synced with main. |
|
||
// TheSuperHackers @feature helmutbuhler 28/04/2025 | ||
// Pass in a csv file to play back multiple replays. The file must be in the replay folder. | ||
{ "-replayList", parseSimReplayList }, |
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.
parseReplayList
@@ -354,6 +354,8 @@ class GlobalData : public SubsystemInterface | |||
std::vector<AsciiString> m_simulateReplays; ///< If not empty, simulate this list of replays and exit. | |||
Int m_simulateReplayJobs; ///< Maximum number of processes to use for simulation, or SIMULATE_REPLAYS_SEQUENTIAL for sequential simulation | |||
|
|||
AsciiString m_writeReplayList; ///< If not empty, write out list of replays in this subfolder into a csv file (TheSuperHackers @feature helmutbuhler 24/05/2025) |
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.
TheSuperHackers epilogue can be omitted for simplicity. It is already marked as such at the Command Line argument entry.
// Call it with -writeReplayList . for all replays in the replay folder. | ||
// Call it with -writeReplayList folder for all replays in the folder subfolder. | ||
// The result will be saved in replay_list.csv in that folder. | ||
// todo: this is a bit unintuitive. Maybe use ReplaySimulation::resolveFilenameWildcards for this? |
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.
What is the plan 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.
I thought maybe we combine -writeReplayList with -replay, so you can call it like:
gen.exe -replay rep1.rep -replay fun*.rep -writeReplayList all_fun_and_rep1.csv
But I havn't thought too much about it. I'm open for suggestions.
// Call it with -writeReplayList folder for all replays in the folder subfolder. | ||
// The result will be saved in replay_list.csv in that folder. | ||
// todo: this is a bit unintuitive. Maybe use ReplaySimulation::resolveFilenameWildcards for this? | ||
{ "-writeReplayList", parseWriteReplayList }, |
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.
I suspect this lives in the client so that maybe there can also be a debug button in the UI to generate this replay list?
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.
Nah, I was just too lazy to make it a separate tool. But we can also add a button for it.
@@ -46,6 +47,11 @@ Int GameMain() | |||
{ | |||
exitcode = ReplaySimulation::simulateReplays(TheGlobalData->m_simulateReplays, TheGlobalData->m_simulateReplayJobs); | |||
} | |||
else if (!TheGlobalData->m_writeReplayList.isEmpty()) |
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.
Why does this live here, inside GameMain, requiring whole GameEngine initialization? Shouldn't this be somewhere else, close to parsing the Command Line?
I suspect TheRecorder
and TheMapCache
are needed. Perhaps we can just create the systems that are needed instead of creating all of them. Tools also do it that way, for example in MapCacheBuilder
.
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.
Yeah, I initially tried to init those things separately, but they kept piling up. You also need TheLocalFilesystem and I think some more. I think it's just simpler to just delay the writing like this.
string->set(*tokenEnd == 0 ? tokenEnd : tokenEnd+1); | ||
} | ||
|
||
bool ReadReplayListFromCsv(AsciiString filename, std::vector<AsciiString>* replayList) |
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.
The CSV reading and writing is embedded with this replay list code. I suggest to decouple these, so that replay list specific code is separate from the code that deals with the CSV format. This will make it easier to understand and maintain this code and reuse the CSV format for other things in the future when so needed.
Here is a simple CSV file for reference: https://github.com/whinc/TinyCSV/blob/master/tinycsv.cpp
Perhaps also decouple the CSV format from the file read/write, so that CSV can also be used with other data sources. I think a simple way to do that is by making the CSV class read and write from iostream-like object (Does not mean it has to be std::iostream, but something that can then be used to read and write from things like std::fstream, std::string, AsciiString, etc. I can help you create that if you like. It should not be much work. AI Chat can also help generate the starting point for this.
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.
I don't think a separate parser is necessary for csv, but if you still want to add it keep in mind there is already some existing code that parses csv files.
if (!fp) | ||
return false; | ||
|
||
// Get list of replay filenames |
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 will be better as a separate function.
|
||
bool ReadReplayListFromCsv(AsciiString filename, std::vector<AsciiString>* replayList) | ||
{ | ||
// Get path of csv file relative to replay folder. |
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 should be a general purpose utility function, like string getPath(filename)
. Can go into FileSystem.
// Some versions, e.g. "Zero Hour 1.04 The Ultimate Collection" have a different ini crc and are | ||
// actually incompatible. Mark them as incompatible | ||
compatibleVersion = compatibleVersion && | ||
(header.iniCRC == 0xfeaae3f3 || header.iniCRC == 0xb859d2f9); |
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 needs to be configurable because Generals would use different ones. Same for the strings. This can be implemented by having an interface class with a pure virtual function and then one derived class in each game that overrides with their custom implementation.
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.
We can also use #if to separate this for each game?
|
||
std::set<int> foundSeeds; | ||
|
||
// Print out a line per filename. i = -1 is csv header. |
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.
I can see why it was implemented like this: to keep header and rows in sync. But this implementation likely will be much cleaner when the header and rows are written seperately. I suspect when the CSV code is moved into its own class, this can be simplified naturally, because this function will then not do multiple things on its own.
Followup for #923
This adds two commandline options:
-writeReplayList
and-simReplayList
. They allow you to write out the current content of the replay folder into a csv file (with some additional information about the replays) and to simulate all the checked replays in a csv file.An example for such a csv is this:
(They can be opened with LibreOffice or Excel)
For writing out these csv-files, there is some commented out code that allows developers to finetune these files if necessary.
These commandline options also work in subfolders of the replay folder.
Right now this PR also contains the diffs of #923. For review you can just look at the last commit (it contains all the changes for this PR)