oscar-broman / samp-weapon-config

A more consistent and responsive damage system with many new features
Apache License 2.0
92 stars 84 forks source link

Problems with y_hooks #185

Closed f0Re3t closed 2 years ago

f0Re3t commented 3 years ago

If you hook the callback via hook function to find out what the next hook/native callback will return, then the hook will not work before weapon-config, and if you register it after connecting weapon-config, the mod will not compile at all.

https://pastebin.com/ynjgTkEm https://pastebin.com/HkD1Vc0h

ADRFranklin commented 3 years ago

I'm not sure what exactly is going on there right now, I will take a look soon and see if I can reproduce it.

NexiusTailer commented 3 years ago

Seems like the problem is widespread enough. Over the past week I saw a couple of similar forum and pm complaints from a few people with their different gamemodes and scripts. There were either similar compiler errors or complaints about the fact that in-hook code is not called for some callbacks when they include weapon-config.

Also, after syncing the changes of the old version of weapon-config with the latest changes in the repo on one of the russian servers when I did an order for them, there were further complaints that some publics from the hook chain stopped being called, which is why I had to roll back the hook code there to the usual ALS method as before and problems stopped happening. Don’t fully understand what could be the cause, but it seems that the new hook code is at least made quite complicated for most of those people who might quickly notice the error after reviewing the changes without long testing.

NexiusTailer commented 3 years ago

As a temporary solution, perhaps it would be reasonable to roll back the changes related to CHAIN_HOOK/CHAIN_ORDER and other macros, making hooks just as ALS method? Or use new code only where it's really needed (OnPlayerDeath/OnPlayerSpawn... ?)

NexiusTailer commented 3 years ago

Sorry for bump, but it seems I found another idea in addition to reverting to classic ALS hooks. If weapon-config returns to using ALS hooks instead of y_hooks, some problems with hook ordering will reappear (any y_hooks-based hooks from any scripts will be called earlier than any ALS hooks, including those in the wc). I can remember at least 2 callbacks where the order of calling it from the hook chain could be important for the weapon-config to have its code executed as soon as possible earlier than anywhere else, and otherwise it could be critical.

But if we move the code from OnPlayerSpawn/OnPlayerDeath to OnPlayerStateChange for PLAYER_STATE_SPAWNED/PLAYER_STATE_WASTED states (OnPlayerStateChange always called BEFORE OnPlayerSpawn/OnPlayerDeath), we could apply weapon-config actions before any other scripts which uses OnPlayerSpawn/OnPlayerDeath and there will be no problems even if people use y_hook-based hooks for those callbacks.

ADRFranklin commented 3 years ago

I just want to point out that we are using ALS hooks, and not y_hooks. If you look at the definitions we are essentially just wrapping the ALS part with a macro and also defining a chain name definition (now this is used by y_prehooks, but doesn't effect the ALS hooks at all)

So just to make it clear, we are not using y_hooks, we are just defining some definitions so that y_prehooks (if included) can pick it up and allow it to setup y_hooks (if included) to hook it. Whether y_hooks or y_prehooks is included or not does not matter since it is still just ALS hooks