Skip to content
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

Merged
merged 12 commits into from
Mar 1, 2024
Merged

Add SaveState class and events #372

merged 12 commits into from
Mar 1, 2024

Conversation

Dregu
Copy link
Collaborator

@Dregu Dregu commented Feb 7, 2024

I guess this copy code works for an arbitrary SaveState class too...

foo = SaveState:new()
prinspect(foo:get_state().time_level)
set_timeout(function()
  foo:load()
  foo:clear()
  foo = nil
end, 60)

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.
Copy link
Contributor

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

Copy link
Collaborator Author

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...

Copy link
Contributor

@Mr-Auto Mr-Auto Feb 8, 2024

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

Copy link
Contributor

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?

Copy link
Collaborator Author

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...

Copy link
Collaborator Author

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.

Copy link
Contributor

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?

Copy link
Collaborator Author

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.

@Dregu Dregu changed the title More SaveState testing Add SaveState class and events Feb 29, 2024
if (!addr)
return;
size_t to = (size_t)(State::get().ptr_main()) - 0x4a0;
auto state = reinterpret_cast<StateMemory*>(addr + 0x4a0);
Copy link
Contributor

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)

Copy link
Collaborator Author

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.

@Dregu Dregu merged commit ab9dd6e into main Mar 1, 2024
10 checks passed
@Dregu Dregu deleted the SaveStates2 branch March 1, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants