Can't hook OnPlayerEnterDynamicCP about ysi-includes

[Crayder]

I know how to get it to compile without errors, but it's not a solution because the script will fail.

I think the new DEFINE_HOOK_REPLACEMENT is faulty.

[wuzi]

You might be right.

After commenting the below code the hook is working.

DEFINE_HOOK_REPLACEMENT(Checkpoint, CP );

(line 160 - /includes/YSI_Coding/y_hooks/impl.inc)

[Y-Less]

The y_hooks code isn't faulty, at least not directly. I think the problem is that the callback really IS called OnPlayerEnterDynamicCP, which y_hooks expands to OnPlayerEnterDynamicCheckpoint, thus making it a callback that is never called.

Basically, it assumes that every instance of CP is an abbreviation for the sake of hook compilation and never assumes that it is used in a shorter version on purpose. "OnPlayerEnterDynamicCheckpoint" is shorter than 32 characters, so is a valid function name, so the logic in the code questions why it would be abbreviated?

So your assertion that the DEFINE_HOOKS_REPLACEMENT code is faulty is wrong - the code does exactly what it was designed to do. The problem is that you don't want it to do it in this circumstance. There are three solutions I can think of:

1. Rename the callback, so it really is called the longer name.

public OnPlayerEnterDynamicCP(playerid, cpid) { CallLocalFunction("OnPlayerEnterDynamicCheckpoint", "ii", playerid, cpid); }

3. Modify y_hooks to somehow know about this case (or rather handle all similar cases in a generic way). The simplest way I can think of to do this is to write TWO entries in the AMX header - one for the fully expanded function name, and one for the exact entered function name, both pointing to the same generated function. The major down-side to this approach is that currently y_hooks will never generate more function entries than already exist. If you generate two for a single hook, it is possible to run out of space in rare cases.

[Y-Less]

So someone just came up with quite a nice(ish) solution for this here:

https://github.com/Misiur/YSI-Includes/issues/75

Based on something written here:

https://github.com/Misiur/YSI-Includes/issues/73

It needs testing, because I can't remember if the replacements are recursive, but one solution might be:

DEFINE_HOOK_REPLACEMENT(DynamicCP, DynamicCP);

Thus replacing the correct version with the still correct version, while marking it as replaced, thus bypassing all the other replacements. Of course, this does hinge on replacements not being recursive...

[Y-Less]

Actually, I've just come up with maybe an even better solution. One that will handle this and all other similar cases neatly. Do the expansion, and check the length of the resulting string. If it is STILL less than 32 characters, then assume that the replacement was not made for the sake of brevity, but was in fact done on purpose, and don't use the new name at all.

[Crayder]

It needs testing, because I can't remember if the replacements are recursive, but one solution might be:

DEFINE_HOOK_REPLACEMENT(DynamicCP, DynamicCP);

Thus replacing the correct version with the still correct version, while marking it as replaced, thus bypassing all the other replacements. Of course, this does hinge on replacements not being recursive...

Actually, I'm pretty sure this does work past the main issue. That was something I tried during https://github.com/Misiur/YSI-Includes/issues/33 and Pottus/Texture-Studio#7 (which are both basically the same issue). It got me past the compiling errors, but weird things occurred when testing them.

[Y-Less]

I've had a play about with the code. My other idea won't work, because from that angle there's no real way to tell a legitimate from an illegitimate replacement. I'll look in to why that like-for-like replacement wouldn't work.

[Y-Less]

OK. There are two things that need to change that I can see. 1) The replacement names need sorting so that the longest ones are always done first. 2) The recursion needs sorting out so that the replaced text is never searched for new replacements. This is actually quite simple since there is explicit code to do that - code which is a bad idea and can frankly be quite simply deleted.

[Y-Less]

New update. I've found some serious flaws in the resorting that y_hooks does for functions! I'll try fix that, then move on to this problem (tomorrow).

[Y-Less]

I used bubblesort because I couldn't be bothered to spend time writing a better sorting algorithm for something that would only be called once on a few strings. I should have bothered - maybe then I would have got it right!