Poll: Change the API?
You do not have permission to vote in this poll.
Yes, break all the plugins
85.71%
6 85.71%
No, keep it as is
14.29%
1 14.29%
Total 7 vote(s) 100%
* You voted for this item. [Show Results]

API change in hook-adding
#1
I think we haven't changed the API for some time now, it's time to break everything (cough coughBig Grin )

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?
Reply
Thanks given by:
#2
It seems realy usefull to be able to split the functions up. I think its worth breaking the plugins.
Reply
Thanks given by:
#3
Are you going to look up which plugin calls the AddHook function by comparing the Lua state pointer?
Reply
Thanks given by:
#4
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)
Reply
Thanks given by:
#5
Oohh, niceBig Grin
Reply
Thanks given by:
#6
Ooh, you're turning all Korean, with their oooooh! Smile
Reply
Thanks given by:
#7
Can you keep the old one but make a new one as well? If not, then just breaking everything is best.
Reply
Thanks given by:
#8
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?
Reply
Thanks given by:
#9
Wow, I've rewritten the hook system almost completely. But it works! Smile 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.
Reply
Thanks given by: NiLSPACE
#10
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);
Reply
Thanks given by:




Users browsing this thread: 1 Guest(s)