(MHRise) Specific hook conditionally causing game freeze

[sigve10]

Describe the bug Creating a hook for snow.player.PlayerSkillList:getSkillData consistently causes freezes when changing in-game equipment, even when there is no logic specified for the hook

Upload logs and any crash dumps re2_framework_log.txt

To Reproduce Steps to reproduce the behavior:

1. Create a hook for the specified method (snow.player.PlayerSkillList:getSkillData).
2. Add any logic you want, or none at all (even specifying the prefunc and postfunc as nil triggers the freeze)
3. Go to the Training Area (found that the freeze most consistently happens here)
4. Open the Item Box, and select Change Equipment
5. Most likely it'll freeze the first time, but if it doesn't, it eventually will if you keep attempting it. The freeze may also occur when changing any equipment piece.

Expected behavior The hook should preferably not freeze the game.

Desktop and VR (please complete the following information):

• OS: Windows 11
• Graphics card: 4095MB NVIDIA GeForce RTX 2060 SUPER
• CPU: AMD Ryzen 5 3600

Other context As much as I'd love to hook onto a different function instead, if this hook were to function, it would save me a lot of manual code and looking through decompilers, since it's the one universal endpoint for the game finding which skills a player has equipped. If this hook does not work, the only other solution as far as I can tell would be to track down specific callers and recreate their logic, which is, needless to say, impractical. I would greatly appreciate any workaround you may come up with.

[praydog]

Unable to replicate. I'm reworking how the hooks work internally though.

[sigve10]

Is it a work in progress change or has it been included in the latest release, nightly or otherwise? I'd be happy to test it again whenever the hook rework is available.

[praydog]

I've pushed my changes for the hooks.

However, I could see this still happening if you're hooking one function, and then calling getSkillData from within the hook, while it's already hooked. Were you doing this previously?

[sigve10]

No, not at all. As mentioned, the freeze occurs for me even without any logic within the hook. I'll test it again tomorrow morning with the new changes.

Granted it's not reproducible though, it might be related to the fact that it's part of a pretty massive project, which already contains a ton of different hooks and functions. I doubt there's a correlation, but maybe there's something in that.

I'll tell you how it goes.

EDIT: Oh wait, I suppose you mean first hooking onto a different function which calls getSkillData in addition to having hooked that function. My mind had immediately hopped to accidental recursion into stack overflowing. I suppose there's a chance I've made a combo like that; As I said, it's a huge and diverse project, so it's not unlikely there's something like that in there somewhere.

[sigve10]

Seems to work properly with the newest version.

Now, I've had some issues in the past where hooks and such would stop working even through game restarts due to some kind of system instability of sorts (I genuinely have no clue either), and those issues would be solved by system restarts. Seeing as how I was testing this on a different system, I gave it a shot with the old version as well in order to see if it wasn't system specific, and sure enough: it did freeze on this system as well. So, that means the new hook system did in fact fix something, so thanks a lot for looking into it! 🎉

[praydog]

I should mention that these changes have removed the guarantee between pre and post that the entire hook will be locked down across threads. Meaning that saving some of the args to use in post may break now if it's a method called on multiple threads.

I'm still playing around with this and thinking of possible solutions that won't break old scripts that were relying on this guarantee.

[praydog]

Nightly release will have a thread API for those instances where you absolutely need something you can only get in the pre hook but need to use it in the post hook for some reason. Many cases this may not matter, but to be absolutely correct, then use the thread API, like this:

local pawn_t = sdk.find_type_definition("app.Pawn")

sdk.hook(
    pawn_t:get_method("updateMove"),
    function(args)
        local storage = thread.get_hook_storage()
        storage["this"] = sdk.to_managed_object(args[2])
    end,
    function(retval)
        local this = thread.get_hook_storage()["this"]
        print("this: " .. tostring(this:get_type_definition():get_full_name()))
        return retval
    end
)

This is only necessary if the hooked function seems to be getting called very frequently, and from multiple threads.

Docs: https://cursey.github.io/reframework-book/api/thread.html

[praydog]

381c67756fd801d9357c34eb98e9cf725256862a should be the final commit that resolves any issues with the new hooking system. Let me know if these new changes still work as intended.

[sigve10]

I'll check it as soon as I have time. Thanks for your hard work! I take it this will become a main branch feature at some point?

[praydog]

It already is on master branch. Unless you mean uploaded to Nexus, then sure. I'm pretty busy with working on the C# API and Dragon's Dogma 2 so don't know when that will happen.

[sigve10]

Ah, I meant the master branch, but it would also be neat to have it on nexus so that people won't have too much trouble getting their hands on it, but there's no haste. The mod won't be out for at least a few months, so it can wait until you have more time on your hands.

I've also gotten to test things now, and I'm glad to see that the whole hook args thing isn't as breaking as it sounded. I'll still upgrade my old functions to use the new thread storage, but it's nice to see that my old mods likely won't be affected. Again, thanks for your hard work!