Server API Revamp

[CoderCow]

Because 1.2 will probably force pretty much any plugin to upgrade, this would be a good opportunity to do have some major upgrades of the server api.

I would like to suggest a revamp of the current hook system, I think the best way to express my thoughts is by showing you a prototype implementation of it. (Imagine all hooks to be defined by the HookManager class in the same way as the NetGetData is implemented. Could also use partial classes if this becomes too messy.)

This is how a plugin would then register a handler: PluginAPI.Hooks.NetGetData.Register(this, this.Net_GetData, 10);

This implementation would have the following advantages:

• Improves stablity because unhandled exceptions are catched for each handler, and if exceptions occur, the handler list invocation will not be aborted (thus one plugin wouldn't prevent other plugins from handling a given hook).
• Lets plugins register each handler with a priority.
• Gives the server more control over the plugin handlers which opens new possibilities in the future. (e.g. invoking of handlers with timeouts to detect deadlocks, deregister handlers and thus disable a plugin automatically if it is malfunctioning etc.)
• Allows a plugin to register a profiler implementation which would easily allow to keep track of the server's performance and especially to detect malfunctioning / bad implemented plugins more easily.
• Allows the server to have a proper log implementation where it can write informative messages to, where this log writer can be overridden by plugins. The server would of course implement a very basic log writer itself, which would then probably be overridden by a TShock log writer implementation once the plugin is loaded.

The only downside I see of this implementation is that the invoking of hooks eats a bit more performance than the old system, considering my (very vague) performance testing on this maybe a few nanosecs more per hook invoke within an average system.

[Olink]

How would this affect plugins that already hook and do massive computations on onupdate? Is the extra time for this implementation linear or exponential?

[Olink]

I guess it should be noted that I am all for a better system, so long as the code is portable between updates (aka we just need to add one-two lines in the new source to invoke the hook etc)

[hakusaro]

I suppose he's got a point though - plugins aren't going to work out of the box anyway.

[CoderCow]

How would this affect plugins that already hook and do massive computations on onupdate? Is the extra time for this implementation linear or exponential?

Wouldn't affect them at all. It's just the call to the handler which takes a very bit more performance than before, the code that is eventually executed by the handler will have no difference in performance. It's basically as with the current implementation, the more handlers register with a hook, the more performance is consumed for each invoke of those, this implementation takes maybe a few nanosecs more processing time for each handler invoke than the current implementation - the overhead is very low and will have no notable effect as long as a hook with maybe 20 handlers is not called 1000 times in one game frame or so.

I guess it should be noted that I am all for a better system, so long as the code is portable between updates (aka we just need to add one-two lines in the new source to invoke the hook etc)

Yes, good point. The Invoke methods should therefore be implemented so that hook invocations are as simple as possible, thus except for a different syntax this design then wouldn't make a difference to the current implementation. In comparison it would be looking like this:

if (NetHooks.OnGetData(ref num2, this, ref index1, ref length))
  return;

vs

if (PluginAPI.Hooks.InvokeNetGetData(ref num2, this, ref index1, ref length))
  return;

So whether a hook is invoked or handlers are registered, it's basically always just one line change. I've also designed it so that adding new hooks is pretty straight forward too.

By the way, another advantage of this system is that it could also deregister all handlers of a plugin automatically when the plugin disposes.

[hakusaro]

:+1:

Lucas Nicodemus via Web

On Sat, Aug 3, 2013 at 1:28 AM, CoderCow notifications@github.com wrote:

How would this affect plugins that already hook and do massive computations on onupdate? Is the extra time for this implementation linear or exponential?

Wouldn't affect them at all. It's just the call to the handler which takes a very bit longer than before, the code that is eventually executed by the handler will have no difference in performance. It's basically as with the current implementation, the more handlers register with a hook, the more performance is consumed for each invoke of those, this implementation takes maybe a few nanosecs more processing time for each handler invoke than the current implementation - the overhead is very low and will have no notable effect as long as a hook with maybe 20 handlers is not called 1000 times in one game frame or so.

I guess it should be noted that I am all for a better system, so long as the code is portable between updates (aka we just need to add one-two lines in the new source to invoke the hook etc)

Yes, good point. The Invoke methods should therefore be implemented so that hook invocations are as simple as possible, thus except for a different syntax this design then wouldn't make a difference to the current implementation. In comparison it would be looking like this:

if (NetHooks.OnGetData(ref num2, this, ref index1, ref length)) return;

vs

if (PluginAPI.Hooks.InvokeNetGetData(ref num2, this, ref index1, ref length)) return;

So whether a hook is invoked or handlers are registered, it's basically always just one line change. I've also designed it so that adding new hooks is pretty straight forward too.

By the way, another advantage of this system is that it could also deregister all handlers of a plugin automatically when the plugin disposes.

— Reply to this email directly or view it on GitHubhttps://github.com/Deathmax/TerrariaAPI-Server/issues/14#issuecomment-22051197 .

[CoderCow]

Alright, I'll prepare a PR for this then, guess another branch for this would be wise?

[hakusaro]

Yes, but if we need to we can always use git fetch and pull your changes directly. Just keep the fork updated and public and such.

[CoderCow]

There we go, first implementation done: https://github.com/CoderCow/TerrariaAPI-Server/tree/1.14

Needs a lot of testing of course, I'll probably port TShock and some other plugins to 1.14 to see how it runs. Feel free to tell me what you think about the code at all and how I can improve it.

[CoderCow]

TShock on 1.14, runs fine: https://github.com/CoderCow/TShock/tree/api_1.14

[hakusaro]

Noblesse oblige. I await your pull request.

Lucas Nicodemus via Web

On Tue, Aug 6, 2013 at 1:58 PM, CoderCow notifications@github.com wrote:

TShock on 1.14, runs fine: https://github.com/CoderCow/TShock/tree/api_1.14

— Reply to this email directly or view it on GitHubhttps://github.com/Deathmax/TerrariaAPI-Server/issues/14#issuecomment-22206534 .

[hakusaro]

This code was added, so I'm closing this pull request.