search

DFHack / dfhack

Memory hacking library for Dwarf Fortress and a set of tools that use it
Other
1.87k stars 475 forks source link

Recent CoreSuspender refactoring altered the behaviour of CoreSuspendClaimer in some situations #1420

Open pronvit opened 5 years ago

pronvit commented 5 years ago

https://github.com/DFHack/dfhack/commit/f6b0ac78196474c99f92c50bc275e4dd5855f3bc @suokko

Previously, CoreSuspendClaimer was effectively a no-op, while now it's an alias to CoreSuspender. This change breaks Remote, which suspends the core a different way while processing its commands. If Remote calls some code in DFHack which in turn uses CoreSuspendClaimer, it will now hang.

Two places this happens for sure is when I'm destroying a Lua-based screen, which uses CoreSuspendClaimer in dfhack_lua_viewscreen::safe_call_lua, and also eventful plugin has it in workshop_hook::interpose_fn_fillSidebarMenu which is called when I simulate a keypress in a workshop.

lethosor commented 5 years ago

I believe @BenLubar ran into something similar with df-ai, and there are now functions in Core to replace what CoreSuspendClaimer did, but I don't remember the exact details at the moment.

lethosor commented 5 years ago

Appears to be a550e112c3307e568309b2bc0c6e026494543e9b - not sure how useful this is for remote.

pronvit commented 5 years ago

That seems to be something else.

lethosor commented 5 years ago

What does df-remote do to suspend the core "differently"?

I'm totally fine with introducing a workaround for df-remote, by the way, but I'm not sure what it requires.

pronvit commented 5 years ago

It directly sends commands to the game loop https://github.com/BenLubar/libgraphics/blob/master/enabler.cpp#L296

I can't use CoreSuspender in Remote because it's too slow in some cases. To maintain simulation FPS, the game processes simulation frames in batches, i.e. processes the required number of frames then waits. DFHack code won't be called until it's time to process the next batch. Instead, I send a command to the game loop and once I get a response, that means it's not inside the simulation code, but most important, the command will wake it up if it's waiting.

I'm not completely sure how the new CoreSuspender works and why the rewrite was required, so maybe @suokko can suggest something. Basically as I said, before CoreSuspendClaimer wasn't actually suspending/waiting (and that was the difference between it and CoreSuspender) and now it is. Another option is to check whether CoreSuspendClaimer is actually required after all the changes.