-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add SaveState class and events #372
Conversation
src/game_api/savestate.hpp
Outdated
class SaveState | ||
{ | ||
public: | ||
/// Create a new temporary SaveState/clone of the main level state. Unlike save_state slots that are preallocated by the game anyway, these will use 32MiB a pop and aren't freed automatically, so make sure to clear them or reuse the same one to save memory. |
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 sounds kind of ominously, like if you don't clear it, you get memory leak
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.
Well I guess the backend could do that for you like for the predefined slots, but we have to be very sure that these are worthless after the level has been unloaded...
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 was thinking more like, if you just set the variable to nil or overwrite it, it will eventually just clear itself (garbage collector), so the clear is not really useful for people who will make like 2/3 slots, only for stuff like TAS that would make a ton of them. So no need to scare people that they need to clear them
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.
Also question: how much did you test the loading save state from previous level? can you just warp to that level and load the saved state or is there something odd that it crashes on?
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.
Well the garbage collector does clear them too, it's just not in any hurry to do that automatically...
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.
Also question: how much did you test the loading save state from previous level? can you just warp to that level and load the saved state or is there something odd that it crashes on?
I have tried replaying the same seed and loading an earlier state in an identical level, still crashed. I don't actually understand why or why not that should work, but that's one reason this option is still left there, for someone to find a way to do it.
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.
Also question: how much did you test the loading save state from previous level? can you just warp to that level and load the saved state or is there something odd that it crashes on?
I have tried replaying the same seed and loading an earlier state in an identical level, still crashed. I don't actually understand why or why not that should work, but that's one reason this option is still left there, for someone to find a way to do it.
So even in seeded it crashes? or are you talking about setting the adventure seed?
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 think I tried both modes, but at least real seeded.
src/game_api/savestate.cpp
Outdated
if (!addr) | ||
return; | ||
size_t to = (size_t)(State::get().ptr_main()) - 0x4a0; | ||
auto state = reinterpret_cast<StateMemory*>(addr + 0x4a0); |
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 actually have a pattern to get this offset 0x4a0
, but if you want to keep it as raw value, can you make it into a global constexpr?
it is also in the State::get() as default (if the pattern would fail for some reason)
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 best would probably be if someone made a struct of the whole heap thingy with prng, state etc in it so we wouldn't need this.
I guess this copy code works for an arbitrary SaveState class too...