Changes for better script online compatibility #369

estebanfer · 2024-02-02T22:38:48Z

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

Mr-Auto · 2024-02-02T22:53:57Z

src/game_api/state.cpp

+// Original function params: clone_heap(ThreadStorageContainer to, ThreadStorageContainer from)
+// HeapContainer has heap1 and heap2 variables, and some sort of timer, that just increases constantly, I guess to handle the rollback and multi-threaded stuff
+// The rest of what HeapContainer has is unknown for now
+// After writing to a chosen storage from the content of `from->heap1`, sets `to->heap2` to the newly copied thread storage


this has to copy more than just state (like prng)
also if you only want callback then figuring out which thread is which from ThreadStorageContainer would be enough, then you could just use detour instead of all of this asm

btw. I specially wrote not long ago patch_and_redirect function, for exactly this kind of assembly hack, so you don't have to write all the boring jumps there and back and copy game code yourself xd

yeah it also copies more stuff, but honestly I don't know how to handle that well in a callback, maybe make a struct for the whole storage that has prng, state, etc.

The issue with figuring out which thread is, is that I don't really know which thread is going to be the destination thread, at the start of the function you only know that to->heap2 pointer is gonna get overwritten at the with a pointer to the destination thread.
For getting to which ThreadStorage will be written to, the game code seems to do some lock and unlock stuff while looping through an array of the threads, which may affect the other threads, so I decided to not try to figure out to which one will be written to with my own code

I'll look into the patch_and_redirect functions

Also, I don't know if I can share some of the ghidra decompiled code to show it

…d by rollback

estebanfer · 2024-03-06T03:30:31Z

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

script state is now in local_state_datas The deadlock was between a thread with g_all_backends_mutex and another with global_lua_lock

Dregu · 2024-03-08T09:12:37Z

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 · 2024-03-19T04:13:30Z

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

Dregu · 2024-03-19T10:44:16Z

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 · 2024-03-19T14:51:22Z

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 · 2024-03-19T15:08:41Z

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 · 2024-03-21T20:59:52Z

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 · 2024-03-21T23:06:07Z

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 · 2024-03-25T02:56:38Z

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 · 2024-03-25T06:54:21Z

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

src/game_api/state.hpp

Mr-Auto · 2024-03-28T09:31:44Z

src/game_api/script/lua_backend.cpp

+    g_state = State::get().ptr_local();
+    if (g_state == nullptr)
+    {
+        g_state = State::get().ptr_main();
+    }
+    ScriptState& state = local_state_datas[g_state].state;


Isn't backend constructed ASAP in playlunky? then this will always be main state anyway

not sure about when they are constructed, are they when you create a new script in OL?

Was thinking more about PL

Yeah but this code is for both PL and OL, also handling what happens in OL makes sense to me

estebanfer · 2024-03-31T03:21:39Z

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 · 2024-03-31T17:25:48Z

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.

src/game_api/state.cpp

estebanfer · 2024-05-03T00:57:27Z

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

src/game_api/script/lua_vm.cpp

g_SpawnTypeFlags is now SYSTEMIC by default because when some threads run spawns for the first times, the SYSTEMIC flag may not be set, causing desyncs online

estebanfer · 2024-08-03T03:52:02Z

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 · 2024-08-03T09:21:31Z

src/game_api/script/usertypes/state_lua.cpp

+    /* StateMemory
+    // user_data
+    // You can store a table (or lua primitive) here and it will store data correctly in online multiplayer, by having a different copy on each state and being copied over when the game does.
+    // Doesn't support recursive tables / cyclic references. Metatables will be transferred by reference instead of being copied


Metatables will be transferred by reference instead of being copied

This is kind of obvious, since you just can't ever do a copy of metatable in lua

metatables are just tables, with fields that are accessed by lua when the table is set as a metatable of one, so those are copiable technically, but copying them doesn't make sense for what they are used most, if not all of the time (which is mostly making OOP like methods for tables). Not sure about metatables created by sol2.
https://www.lua.org/pil/13.html

Mr-Auto · 2024-08-03T09:28:43Z

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 · 2024-08-03T20:25:29Z

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

Mr-Auto · 2024-08-03T20:30:07Z

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

WIP rollback function hook

0c65be3

estebanfer marked this pull request as draft February 2, 2024 22:39

Mr-Auto reviewed Feb 2, 2024

View reviewed changes

estebanfer added 3 commits February 9, 2024 17:51

add local_data to StateMemory, to allow script data that gets affecte…

5a590a5

…d by rollback

hook rollback function with patch_and_redirect and fix it

2543422

Merge branch 'main' into online-compat-stuff

9a994e4

estebanfer added 2 commits March 6, 2024 02:24

Merge branch 'main' into online-compat-stuff

bbffae5

Move local_state_datas to LocalStateData struct

a91419c

estebanfer force-pushed the online-compat-stuff branch from dbe6e4a to a91419c Compare March 6, 2024 05:49

estebanfer added 3 commits March 7, 2024 12:27

Merge branch 'main' into online-compat-stuff

f2e4f8b

make more callbacks online compatible, fix deadlock

e70fa55

script state is now in local_state_datas The deadlock was between a thread with g_all_backends_mutex and another with global_lua_lock

format

79c13f3

Mr-Auto reviewed Mar 25, 2024

View reviewed changes

src/game_api/state.hpp Outdated Show resolved Hide resolved

estebanfer mentioned this pull request Mar 28, 2024

StateMemory and State structs changes #379

Draft

estebanfer force-pushed the online-compat-stuff branch from c364fa9 to 79c13f3 Compare March 28, 2024 04:47

Mr-Auto reviewed Mar 28, 2024

View reviewed changes

estebanfer added 2 commits March 29, 2024 00:26

docs, rename clone_heap -> copy_state, state local_data -> user_data

43386cb

change state location to read-only via getter

8df6529

trigger PRE_COPY_STATE on Overlunky savestates

488d643

Mr-Auto reviewed Apr 10, 2024

View reviewed changes

src/game_api/state.cpp Outdated Show resolved Hide resolved

estebanfer added 3 commits April 10, 2024 17:17

move State::get_location to header file

c704236

minor autodoc fix

3881f06

remove PRE_COPY_STATE lua event for now

7e9c411

estebanfer marked this pull request as ready for review May 1, 2024 02:06

fix get_local_* return type in docs

f105e88

estebanfer mentioned this pull request May 6, 2024

Online scripts compatibility stuff pt. 2 #382

Open

Mr-Auto reviewed Jun 2, 2024

View reviewed changes

src/game_api/script/lua_vm.cpp Outdated Show resolved Hide resolved

fix spawn_api and custom_allocator online

f93ee54

g_SpawnTypeFlags is now SYSTEMIC by default because when some threads run spawns for the first times, the SYSTEMIC flag may not be set, causing desyncs online

estebanfer mentioned this pull request Jun 26, 2024

Use Vec2 more, some small issues fix, more unknown stuff about camera and Hud #387

Merged

estebanfer added 2 commits August 3, 2024 00:25

Merge branch 'main' into online-compat-stuff

4b1a8b2

better deepcopy_object implementation

98cb792

Mr-Auto reviewed Aug 3, 2024

View reviewed changes

change get_location to get_offset, remove get_state_offset

9f70235

estebanfer merged commit 1851abd into spelunky-fyi:main Dec 4, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes for better script online compatibility #369

Changes for better script online compatibility #369

estebanfer commented Feb 2, 2024

Mr-Auto Feb 2, 2024

estebanfer Feb 2, 2024 •

edited

Loading

estebanfer Feb 2, 2024

estebanfer commented Mar 6, 2024

Dregu commented Mar 8, 2024

estebanfer commented Mar 19, 2024 •

edited

Loading

Dregu commented Mar 19, 2024

Mr-Auto commented Mar 19, 2024

Dregu commented Mar 19, 2024

estebanfer commented Mar 21, 2024

Dregu commented Mar 21, 2024

estebanfer commented Mar 25, 2024

estebanfer commented Mar 25, 2024 •

edited

Loading

Mr-Auto Mar 28, 2024

estebanfer Mar 29, 2024

Mr-Auto Jun 2, 2024

estebanfer Aug 10, 2024

estebanfer commented Mar 31, 2024

Dregu commented Mar 31, 2024

estebanfer commented May 3, 2024

estebanfer commented Aug 3, 2024

Mr-Auto Aug 3, 2024

estebanfer Aug 3, 2024

Mr-Auto commented Aug 3, 2024

estebanfer commented Aug 3, 2024

Mr-Auto commented Aug 3, 2024

Changes for better script online compatibility #369

Changes for better script online compatibility #369

Conversation

estebanfer commented Feb 2, 2024

Choose a reason for hiding this comment

estebanfer Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estebanfer commented Mar 6, 2024

Dregu commented Mar 8, 2024

estebanfer commented Mar 19, 2024 • edited Loading

Dregu commented Mar 19, 2024

Mr-Auto commented Mar 19, 2024

Dregu commented Mar 19, 2024

estebanfer commented Mar 21, 2024

Dregu commented Mar 21, 2024

estebanfer commented Mar 25, 2024

estebanfer commented Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estebanfer commented Mar 31, 2024

Dregu commented Mar 31, 2024

estebanfer commented May 3, 2024

estebanfer commented Aug 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mr-Auto commented Aug 3, 2024

estebanfer commented Aug 3, 2024

Mr-Auto commented Aug 3, 2024

estebanfer Feb 2, 2024 •

edited

Loading

estebanfer commented Mar 19, 2024 •

edited

Loading

estebanfer commented Mar 25, 2024 •

edited

Loading