spelunky-fyi / overlunky

An overlay for Spelunky 2 to help you with modding, exploring the depths of the game and practicing with tools like spawning arbitrary items, warping to levels and teleporting.
MIT License
83 stars 31 forks source link

PauseAPI #366

Closed Dregu closed 8 months ago

Dregu commented 9 months ago

PauseAPI can be accessed from the global pause, which can also be called to keep the old functionality.

-- pauses game loop when level timer reaches 60
set_callback(function()
  set_callback(function()
    if state.time_level == 60 then
      prinspect("PRE Pausing the game loop", state.time_level, get_global_frame())
      pause:set_pause(PAUSE_TYPE.PRE_GAME_LOOP)
      clear_callback()
    end
  end, ON.PRE_GAME_LOOP)

  local foo = set_callback(function()
    prinspect("POST Game loop is still running", state.time_level, get_global_frame())
  end, ON.POST_GAME_LOOP)

  set_callback(function()
    prinspect("BLOCKED Blocked game loop", state.time_level, get_global_frame())
    clear_callback(foo)
    clear_callback()
  end, ON.BLOCKED_GAME_LOOP)
end, ON.PRE_LEVEL_GENERATION)
Cosine256 commented 9 months ago

Are these changes ready enough to be tested? I can try to integrate TASW with this new API and see if I run into any issues.

Dregu commented 9 months ago

Are these changes ready enough to be tested? I can try to integrate TASW with this new API and see if I run into any issues.

I'm just cleaning up and writing docs, so maybe in 30 minutes.

Dregu commented 9 months ago

Well does it work @Cosine256? The old ways or the new.

Cosine256 commented 9 months ago

Sorry, I still haven't had time to properly test this with TASW yet. I'll comment here when I've done so.

Cosine256 commented 8 months ago

I apologize about how long it took me to take a look at this.

I assume the goal here is to make sure the API has the necessary features so there don't need to be breaking changes later. I mostly just tested pause features relevant to TASW. I set up a version of TASW that uses the new pause API. This seems to work way better than the bucket options and I was able to completely switch over to the new pause object instead. It's looking like TASW will work best with PRE_UPDATE (to let PauseAPI run logic during fast updates), PRE_GAME_LOOP, and PRE_PROCESS_INPUT freezes all together, so that's what I focused my testing on. I also tested the current released TASW with fade pauses and didn't notice any issues.

These are the issues and questions I have right now:

  1. Pause triggers don't fire reliably during batches of update_state() calls. I think it's an issue with checking the frame count in the trigger logic. Stuff like the fade timer might get changed by update_state(), but since the frame count didn't increase, the pause API will ignore the trigger.
  2. How are modifiers_down, modifiers_block, and modifiers_clear_input related to PauseAPI specifically? They seem like a generic feature that could apply to any hotkey, not just pauses. Do they belong in PauseAPI? I had issues with setting up hotkeys when I was initially planning to build all of this pause stuff into TASW itself. It felt like I needed some kind of hotkey API. It was stuff like game inputs not being blocked for hotkeys, hotkeys triggering while focused on ImGui text inputs, no good way to capture a hotkey like OL, no good way to display a hotkey by name, etc. That's all quite out of scope for PauseAPI, but I'm bringing it up now because putting this modifier logic in PauseAPI and then moving it later would be a breaking change.
  3. I don't currently have PL set up to build and test this branch myself. All of this PauseAPI stuff still exists with just PL? It gets overwritten by OL's saved options when OL is attached? The default pause type is PRE_UPDATE for just PL alone, with all the other triggers and ignores turned off?
Dregu commented 8 months ago

Pause triggers don't fire reliably during batches of update_state() calls. I think it's an issue with checking the frame count in the trigger logic. Stuff like the fade timer might get changed by update_state(), but since the frame count didn't increase, the pause API will ignore the trigger.

Oh, I think I was under the illusion that update_state was the one incrementing the frame count, but I guess it only copies the frame count to time_startup. It's been a few weeks too long to remember why all the checks exist exactly, but adding one more counter for any sort of state updates specifically might fix this then. Some kind of same update check is needed anyway, or the trigger will just keep pausing for the same reason after manual unpause.

How are modifiers_down, modifiers_block, and modifiers_clear_input related to PauseAPI specifically?

It's pretty related if you think that the feature is just pausing, or rather blocking process_input, which is also one of the possible pause types in PauseAPI. It has some extra functionality on top of that, but I don't see toggling input pause on keypress not that different from toggling update pause on screen change. It's not going to move anyway.

I don't currently have PL set up to build and test this branch myself. All of this PauseAPI stuff still exists with just PL?

Yes. Worth mentioning about the OL-PL-compatibility again, because PL hook functions will be treated as the vanilla functions by OL, the promises of all PRE and BLOCKED callbacks executing still don't apply to PL when both tools are running and OL is blocking it. If PL toggles PRE_UPDATE pause in PRE_UPDATE, it would get blocked by PL-PauseAPI once, then OL would take over blocking on the next cycle and no further *_UPDATE callbacks are possible in PL while that pause is on. If that hurdle is too big to design around, it could be possible to add an option to only execute PauseAPI on the last tool in the chain (PL), but still have shared options and use keys from OL. The tools do know if they're hooking vanilla game or not.

It gets overwritten by OL's saved options when OL is attached?

Yes. OL will overwrite the pause options with ini options when attached, but the checkboxes are directly bound to the shared options in bucket, so changes in the api are updated to the UI immediately too. I could also add functionality to detect what tool the script is running in, all the tools that are running and a callback in PL when OL was loaded, if that would help sorting this problem.

The default pause type is PRE_UPDATE for just PL alone, with all the other triggers and ignores turned off?

Yes. A default pause type mainly exists to not completely break the old pause(true) function. I should make sure everything else is initialized to 0, but that's the idea.

Cosine256 commented 8 months ago

Oh, I think I was under the illusion that update_state was the one incrementing the frame count, but I guess it only copies the frame count to time_startup. It's been a few weeks too long to remember why all the checks exist exactly, but adding one more counter for any sort of state updates specifically might fix this then. Some kind of same update check is needed anyway, or the trigger will just keep pausing for the same reason after manual unpause.

Do you intend to fix that in this PR? I know I let this sit for like a month, so no worries if you don't get to it immediately. I could probably also work around it in TASW if needed and warn users that pause triggers aren't reliable during fast updates.

Yes. Worth mentioning about the OL-PL-compatibility again, because PL hook functions will be treated as the vanilla functions by OL, the promises of all PRE and BLOCKED callbacks executing still don't apply to PL when both tools are running and OL is blocking it. If PL toggles PRE_UPDATE pause in PRE_UPDATE, it would get blocked by PL-PauseAPI once, then OL would take over blocking on the next cycle and no further *_UPDATE callbacks are possible in PL while that pause is on. If that hurdle is too big to design around, it could be possible to add an option to only execute PauseAPI on the last tool in the chain (PL), but still have shared options and use keys from OL. The tools do know if they're hooking vanilla game or not.

This sounds like a mess, though I might be able to work around it in TASW. Is this problem just a consequence of how there are two copies of the script API attached to a single process if you're running PL and OL together? I had always wondered how this case was handled even before working on TASW, but never had to deal with it in a script.

Would things work better if there was only ever one instance of the script API? It seems like that's the direction these changes are going in now that there is a shared bucket. OL could be able to detect this and use the instance attached by PL. And without its own script API, isn't OL basically a huge mod compiled in C++ instead of written in Lua? I can only think of a few other things OL does that can't be done by a PL script. OL can attach to a running process at any time, it can enable/disable mods itself, uses some unexposed ImGui calls, and maybe some of the cheat options aren't currently exposed in the script API. I'm not specifically suggesting rewriting OL as a Lua mod, but rather having it behave like a Lua mod as far as the script API is concerned for callbacks and game cheats.

I'm sure doing anything like this is a ton of work, and TASW might be one of the only mods where it even matters right now. I understand if nobody wants to spend that much timing working on it. I'll see what I can do in TASW to work around any PL/OL issues you don't fix in this PR. This pause API is already a huge improvement over what I was working with before.

Yes. OL will overwrite the pause options with ini options when attached, but the checkboxes are directly bound to the shared options in bucket, so changes in the api are updated to the UI immediately too. I could also add functionality to detect what tool the script is running in, all the tools that are running and a callback in PL when OL was loaded, if that would help sorting this problem.

This question was just me trying to make sure I understood when the pause options could get changed. I don't think I need any additional changes here since it works how I expected it to. The new TASW will have an option to force its recommended pause type on each GUI frame, so that would immediately set the pause API back to the desired pause type if OL had a different one saved. There are some problems with getting stuck if I change the pause type while a pause is active using state.pause flags, but since those are considered obsolete, I've decided not to worry about them right now.

Dregu commented 8 months ago

Do you intend to fix that in this PR?

At least that yes, probably in a few days.

Is this problem just a consequence of how there are two copies of the script API attached to a single process if you're running PL and OL together?

Yeah totally just a side effect of two instances and it just works magically, cause the patterns that search for game functions to hook are designed to still work when one instance is already attached, but the game code instead contains a detour to playlunky code, which then gets chained with a detour to overlunky code. It's great that it works, but might be tricky work around in the case that the first tool wants to block something, but the second needs to know about it.

In this specific PauseAPI case it might be pretty easy to work around, since we already know both instances will block and maybe I can make it work more generally with all BLOCKED callbacks, so all instances would always get those. I'll see what I can do this week about that.

Would things work better if there was only ever one instance of the script API?

Yeah sure, but then again this blocking PRE callbacks behavior and string edits getting negated by the other instance are maybe the only problems with multiple instances.

I'm sure doing anything like this is a ton of work, and TASW might be one of the only mods where it even matters right now. I understand if nobody wants to spend that much timing working on it.

Yeah for those reasons nobody probably will do much about it, but some workarounds will be easier to add.

Dregu commented 8 months ago

I don't if this all works, and it's been too long of a day to know if it's even a good idea, but I

These are two separate solutions to the same problem, but the result should be the same when running TASW in PL with OL injected: PRE/POST/BLOCKED_UPDATE/GAME_LOOP/PROCESS_INPUT callbacks still run.

Cosine256 commented 8 months ago

The pause triggers work properly during TASW fast updates now. I haven't found any more issues with this PauseAPI PR. I checked both the live version of TASW and the one that uses the new pause features. The only thing I didn't test was the PL/OL interactions. I still haven't tried to set up PL locally, though I can figure out how to do it if you think there is a decent risk that the blocked callback stuff doesn't work.

I have some more changes to make in my TASW update, so I wouldn't be releasing it in parallel with this OL update right now. If you don't want to wait for that (understandably), then I think this PR is fine and at least won't need any API changes. If I find anything I missed, then I'll still report it, but I can probably work around it and won't need a quick fix. I appreciate what you've done here already. This fixed all of the "can't pause on frame X" problems with TASW, and made pausing and frame advance way nicer to interface with.

Cosine256 commented 8 months ago

I found an issue where I'm getting dropped inputs:

  1. Set pause flags to freeze updates, game loop, and inputs.
  2. Go to camp.
  3. Activate a freeze pause.
  4. Tab out of the game window (I'm using the borderless window setting).
  5. Tab back into the game window.
  6. Hold any combination of inputs and use frame advance once (I'm using keyboard).

Those first inputs are ignored by the game during that frame advance, and will keep being ignored until released. Any newly held inputs during subsequent frame advances are processed correctly, but the first set of inputs keeps getting dropped. Releasing the first set of inputs fixes them, even if you don't frame advance while they're released.

This is happening on both the PauseAPI branch and the OL nightly (472a095). I don't know when this issue was introduced. My work on TASW only used fade pauses until recently, so perhaps it's an issue with how the freeze pauses were implemented and it got carried over into PauseAPI?

Dregu commented 8 months ago

I suppose that only happened when freeze input was enabled? I guess that needs to run after focus loss to reinitialize the input somehow. I don't really know what it's doing, but I think allowing it to run once when gaining focus fixed it?

Dregu commented 8 months ago

I removed BLOCKED_LEVEL_GENERATION and made POST_L_G run even when PRE_L_G is blocked, cause it didn't make sense and in the end a level will always exist anyway, maybe just with less entities. If PRE_L_G was blocked in one mod just to use a custom level format (what blockable PRE_L_G is actually for), skipping POST_L_G would make any other mods relying on it incompatible.