Replay system part 1 #379
Replay system part 1 #379
Conversation
Theoretically, the scenario and character editors could also have replay systems
| auto srand_element = pop_next_action("srand"); | ||
|
|
||
| std::string ts(srand_element->GetText()); | ||
| srand(atoi(ts.c_str())); |
There was a problem hiding this comment.
Seeing this made me realize that we may want to consider moving away from rand() in favour of a Mersenne twister… but that would be something for a separate PR.
src/game/boe.main.cpp
Outdated
| if (file.empty()) | ||
| file = "BoE"; | ||
|
|
||
| if (boost::ends_with(file, ".xml")) |
There was a problem hiding this comment.
I think Boost.Algorithm is a new dependency, right? I don't mind adding it, but it needs to be mentioned in the README.
There was a problem hiding this comment.
It didn't need to be added to the setup of the CI for any of the platforms--it must be a dependency of one of the packages we already install.
The readme says
- Boost - Filesystem and System, plus several header-only
libraries; if you're picky, you can run scons and see it enumerate exactly which
libraries are needed
So I think it could fall under that umbrella.
There was a problem hiding this comment.
Nice to see that the readme is contradicting itself slightly – the vcpkg instructions do enumerate every necessary Boost library, after all. (Though I'm not confident there are no others missing.)
|
I must have missed something in the rebase, I think I had that fixed
…On Sat, Jul 27, 2024 at 5:00 PM Celtic Minstrel ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/tools/prefs.win.cpp
<#379 (comment)>:
> @@ -99,10 +101,18 @@ static bool save_prefs(fs::path fpath) {
static bool load_prefs(fs::path fpath) {
prefsDirty = false;
std::map<std::string,boost::any> temp_prefs;
- std::ifstream fin(fpath.string().c_str());
+ std::istream* in;
This should be std::unique_ptr<std::istream>. (If that causes problems
for some reason, shared_ptr is also acceptable.)
—
Reply to this email directly, view it on GitHub
<#379 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AATXBKIOBCHKE3YBQJFUGJLZOQKBNAVCNFSM6AAAAABLSGI2Q6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMBTGIZTQMRVHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
src/game/boe.main.cpp
Outdated
| if(t == "load_party") { | ||
|
|
||
| if(overall_mode == MODE_STARTUP && t == "startup_button_click"){ | ||
| eStartButton btn = static_cast<eStartButton>(atoi(next_action->GetText().c_str())); |
There was a problem hiding this comment.
I'm not going to consider this a blocker, but I think it would be better to use the estreams.cpp conversion mechanism here, giving a string representation for each enum value.
|
Sorry for the false alarm, I was going through it one commit at a time. I think the only thing missing then is that as far as I know there's no documentation that Boost.Algorithm is now used. |
|
I pulled this and verified that it builds on Xcode 12, so I think there's no major issues. (I'd like it to be able to build on Xcode 3, mind you, but I'm not able to verify that right now.) |
|
Another thing I was thinking which is probably better for another PR is that we should probably use a proper command line processing library. Given our existing dependencies, the most logical choices in my mind are:
That said, I would be open to other possibilities as well. It appears you've chosen to use a subcommand setup (similar to git), and I'm not sure if either of the above libraries can handle that sort of thing. (Boost is more likely to be able to, but I haven't looked.) |
|
Okay, I think I'm going to go ahead and merge this, but you certainly should add Boost.Algorithm to the README at some point (in the vcpkg install line, maybe also in the place that currently can't be bothered to enumerate the dependencies). |
This is rebased with all the little things fixed.