Changes for better script online compatibility

[estebanfer]

For now it only has a WIP code for the rollback function hook that copies the data from one thread to another, it works but I still have to make it better, add more changes and decide how to name the heap clone functions

[estebanfer]

Should I rename the PRE_CLONE_HEAP event now that there's the other events? (LOAD_STATE, SAVE_STATE)

[Dregu]

Should I rename the PRE_CLONE_HEAP event now that there's the other events? (LOAD_STATE, SAVE_STATE)

Yeah I think the best thing moving forward would be to just call it "state" (so CLONE_STATE or COPY_STATE), grow the existing struct to start at offset 0, renaming it to "State" in the api, absorbing whatever there is before the current StateMemory and possibly after (whatever is part of this heap), making sure the globals always point to the local versions...

[estebanfer]

Would StateMemory be merged with the current existing State struct? Or you refer to something else with the existing struct?

[Dregu]

I meant grow the currently existing StateMemory struct from the beginning by 0x4a0, and rename it State. I know we have a State struct already, but parts of it really have nothing to do with the state of anything and is just for initializing the script api. Rest is a random collection of api functions and stupid getter functions to read stuff from just before the StateMemory, you know, stuff that would be obsolete when the new State(Memory) actually covers that memory.

So

• rename the current State to ScriptAPI or whatever, keeping the relevant functions, removing the soon obsolete functions
• grow the current StateMemory by 0x4a0 and rename it State

or

• split the current State to ScriptAPI and State
• put old StateMemory inside this new State

and

• fill the beginning of new State with stuff we know is in it, like the frame count and prng
• organize the functions so new State only has functions that deal with that State, rest somewhere else

Now I don't exactly know what else is in the beginning of that new State, but there's clearly about 0x4a0 bytes of stuff that we should reverse engineer.

[Mr-Auto]

grow the current StateMemory by 0x4a0 and rename it State

put old StateMemory inside this new State

Those two points are mutually exclusive

There is also a ton of stuff after StateMemory, including a lot of the inner structs of StateMemory, level gen, liquid physics, but also save data etc.

I don't really see a reason for one giant struct, it will probably just mess with compile time or you will end up fighting the compiler alignments in it.

For the x64dbg plugin i just made one function to get heap address and then you get offset for specific thing via enum

[Dregu]

Those two points are mutually exclusive

Yes that's why I put "or" between them.

Anyway what we're doing currently with the stuff before StateMemory (get StateMemory* and subtract some random offset) is absolutely bonkers compared to any alternative.

[estebanfer]

I was going with the option of growing state, but not I noticed we have some game functions that use StateMemory* with the current size, like spawn_companion, is_inside_active_shop_room, update_state, and some hooks... So I guess I'm going for the second option by putting StateMemory in State, but would the lua API now use StateMemory or State?

[Dregu]

I was going with the option of growing state, but not I noticed we have some game functions that use StateMemory* with the current size, like spawn_companion, is_inside_active_shop_room, update_state, and some hooks... So I guess I'm going for the second option by putting StateMemory in State, but would the lua API now use StateMemory or State?

Well yeah clearly StateMemory is its own thing cause it's referenced like that a lot, but also clearly part of a slightly larger thing, so second option makes more sense. We should really figure out what all the stuff in that 0x4a0 even is, but on the lua side things don't really have to change, as we have getters for all the known things in there anyway. Things can be added as properties of lua["state"] even if not exactly in StateMemory though, if that makes sense.

Even if we end up with just this, it's better than the magic number getters we currently have:

struct State { // offset 0
  // padding
  PRNG prng;
  uint32_t frame_count;
  // padding
  StateMemory state; // offset 0x4a0
};

Still open for suggestions for the naming of those structures, but internally it doesn't really matter that much.

[estebanfer]

Things can be added as properties of lua["state"] even if not exactly in StateMemory though, if that makes sense.

Wouldn't that require doing the same as before, substracting an offset to get the stuff in State?

Still open for suggestions for the naming of those structures, but internally it doesn't really matter that much.

Having stuff like state->state doesn't sound good to me, though I don't have many ideas. Seems that we already have some name for it from before, the OnHeapPointer class in thread_utils.hpp, but idk why it was named like that

[estebanfer]

I made most or all changes, though I had some doubts, I decided just to do the changes and then get it reviewed

[estebanfer]

I have a local stash of changes to make timers and callbacks started from other callbacks to work online, by moving the callbacks unordered_map and other fields from LuaBackend to LocalStateData and creating custom structs that only get copied when they have changes, so it has many lines of code edited. Should I commit that here or make another PR, to prevent this one from growing too much?

Also, should I make PRE_COPY_STATE to be also called from the savestate functions? Makes sense to me since people using PRE_COPY_STATE probably use it for online mods and wouldn't use the other save callbacks, and also being able to test a part of the rollback without having to open an online game seems be nice

[Dregu]

I have a local stash of changes to make timers and callbacks started from other callbacks to work online, by moving the callbacks unordered_map and other fields from LuaBackend to LocalStateData and creating custom structs that only get copied when they have changes, so it has many lines of code edited. Should I commit that here or make another PR, to prevent this one from growing too much?

Yeah maybe we should do this in smaller parts, but I'll say I have no intention of actually testing any of this online, just to make sure some single player mods still works right.

Also, should I make PRE_COPY_STATE to be also called from the savestate functions? Makes sense to me since people using PRE_COPY_STATE probably use it for online mods and wouldn't use the other save callbacks, and also being able to test a part of the rollback without having to open an online game seems be nice

Makes sense to me too, even if just for testing.

[estebanfer]

I removed the PRE_COPY_STATE for now because I don't really know what someone could use it for, so I prefer to leave it out until having something more clear

[estebanfer]

On this PR I added get_location to State, but now there is also get_state_offset in savestate.cpp which does the same. Which one should be preferred?

[Mr-Auto]

On this PR I added get_location to State, but now there is also get_state_offset in savestate.cpp which does the same. Which one should be preferred?

I don't think that matters much. Just pick one

[estebanfer]

do you mean to let anyone decide which to use, or to remove one of those?

[Mr-Auto]

do you mean to let anyone decide which to use, or to remove one of those?

Aren't they exactly the same? it doesn't really matter which one we use. So just remove one. Also this is really not a function that's used often, so it doesn't matter.

Also also, i plan to kill this function anyway, since we can just hardocde the offset 4A0 , it's not a big deal