Skip to content

Fix SDKHook_[Use|Spawn|GetMaxHealth] callback result value handling#1872

Merged
peace-maker merged 3 commits intomasterfrom
sdkhooks_retval
Dec 4, 2022
Merged

Fix SDKHook_[Use|Spawn|GetMaxHealth] callback result value handling#1872
peace-maker merged 3 commits intomasterfrom
sdkhooks_retval

Conversation

@peace-maker
Copy link
Member

The returned result of the last callback in the list was used instead of the highest value. This differs from the behavior of the other hooks.

The GetMaxHealth hook has an outparam. I've changed the return value handling to only return the changed value if Pl_Changed was the highest value a plugin wanted. If some plugin returned Pl_Handled it returns the original value. This is similar to the TraceAttack hook handling.

Fixes #1870

The returned result of the last callback in the list was used instead
of the highest value. This differs from the behavior of the other hooks.
The returned result of the last callback in the list was used instead
of the highest value. This differs from the behavior of the other hooks.
The returned result of the last callback in the list was used instead
of the highest value. This differs from the behavior of the other hooks.
The returned health is only changed if no other plugin wants to block the callback.
@KyleSanderson
Copy link
Member

Hopefully wasn't when I did the vectorization. Are you sure this goes far enough? Plugin_Stop doesn't appear to be implemented.

There was a patch for this to use fwdsys, but preserving the behaviour in one of the callbacks wasn't consistent so I think it died in a branch.

@peace-maker
Copy link
Member Author

You're right, I didn't touch breaking the loop on Plugin_Stop yet. I wanted to check how fwdsys does it first.

Do you remember which hook was a special snowflake when switching to private forwards?

@KyleSanderson
Copy link
Member

KyleSanderson commented Dec 3, 2022

You're right, I didn't touch breaking the loop on Plugin_Stop yet. I wanted to check how fwdsys does it first.

Do you remember which hook was a special snowflake when switching to private forwards?

I think it's all of them with an action that doesn't break in a loop (I just looked, it's a fairly sizeable chunk). Right now you can return Handled or Stop and it will still spin through all the callbacks. The concession in 2014~ was that there's already too much code out there to tolerate the breakage. There may have been something else as well where Handled was False and Stop was true, but it's cold and I didn't see it.

(Et_Ignore Vs Et_Event still was a behaviour change)

@peace-maker peace-maker merged commit aab8c6a into master Dec 4, 2022
@peace-maker peace-maker deleted the sdkhooks_retval branch December 4, 2022 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SDKHook_Use doesn't respect return Plugin_Handled.

2 participants