API change in hook-adding

[xoft]

I think we haven't changed the API for some time now, it's time to break everything (cough cough )

I'd like to propose a change that could make life somewhat easier for plugin authors. This change would allow them to modularize their plugins and possibly to merge multiple plugins into one (as is lately the case of the Core).

The change is subtle at first look: go from

cPluginManager:Get():AddHook(PluginInstance, HOOK_TYPE);

to

cPluginManager:Get():AddHook(HOOK_TYPE, HandlerFunction);

Two changes here. You no longer need to provide the PluginInstance variable. Yay! It was useless anyway!
Second, you provide a handler function directly, so you can name it whatever you want. Right? Right.

And even better than that - you can add multiple functions to be called for a single hook. Why would you need that? To illustrate, let's examine the Core. It currently provides SpawnProtect and LimitWorld functionality. Both of these require that an OnPlayerPlacingBlock() handler is provided, so that the player may not place blocks inside the spawn area and outside the world limit. So the code for both needs to be intertwined together in a single OnPlayerPlacingBlock() function. A Big Ball Of Wibbly Wobbly Timey Wimey ... Stuff.
Now, if each "functionality module" could add their own hook, you could split it and have SpawnProtect implemented in a single Lua file, and LimitWorld in another single Lua file, the only thing in common would be a call to each respective InitializeXXX function in the common plugin Initialize() function. Modularity, finally!

What do you say, is this change worth breaking things?

[NiLSPACE]

It seems realy usefull to be able to split the functions up. I think its worth breaking the plugins.

[FakeTruth]

Are you going to look up which plugin calls the AddHook function by comparing the Lua state pointer?

[xoft]

I'm gonna use the exact same mechanism that cPluginManager:BindCommand() already uses. There's a global variable in each lua_State holding the pointer to the plugin (GetLuaPlugin() function in ManualBindings.cpp)

[FakeTruth]

Oohh, nice

[xoft]

Ooh, you're turning all Korean, with their oooooh!

[bearbin]

Can you keep the old one but make a new one as well? If not, then just breaking everything is best.

[xoft]

Now that you mention it, yes, it is possible to keep the old way, too. That would be the best way to do it. It'll be a bit more work to write it correctly, but it should work. The binding function will have to check parameter types, and if it detects old-style signature, it can provide the old behavior (with a warning to the console). Why didn't I think of it sooner?

[xoft]

Wow, I've rewritten the hook system almost completely. But it works! I just hope I didn't make some stupid mistake in there, go ahead and test it.

Don't forget, the recommended way of adding hooks is

cPluginManager.AddHook(HOOK_TYPE, CallbackFunction);

Note the dot instead of a colon, the AddHook function is "static" - it doesn't need a cPluginManager instance (since there's only one anyway). Colon will be accepted, too, but is deprecated.

---

I'll update the HookNotify and the Core plugins for the new API now.

[NiLSPACE]

Maybe also an idea is to export the hooks more like items. Instead of

cPluginManager.AddHook(cPluginManager.HOOK_TICK, CallbackFunction);

do

cPluginManager.AddHook(HOOK_TICK, CallbackFunction);