Fix sourcehook compilation warning

[Kenzzer]

On newer version of clang the compiler complains about a missing virtual dtor. So let's just add one ?

/home/kenzzer/github/CBaseNPC/extension/toolsnextbot.cpp:7:1: error: delete called on non-final '__SourceHook_MFHCls_ToolsNextBot_OnTakeDamage_Alive::CMyDelegateImpl' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non-abstract-non-virtual-dtor]
SH_DECL_MANUALHOOK1(ToolsNextBot_OnTakeDamage_Alive, 0, 0, 0, int, const CTakeDamageInfo&);
^
/mnt/c/alliedmodders/metamod-source/core/sourcehook/sourcehook.h:1439:3: note: expanded from macro 'SH_DECL_MANUALHOOK1'
                MAKE_DELEG(rettype, (param1 p1), (p1)); \
                ^
/mnt/c/alliedmodders/metamod-source/core/sourcehook/sourcehook.h:1047:23: note: expanded from macro 'MAKE_DELEG'
                void DeleteThis() { delete this; } \ \

[psychonic]

This change will eventually be lost if not put into core/sourcehook/generate/sourcehook.hxx as well. sourcehook.h is meant to be a generated file.

[Kenzzer]

Would this be correct ?

[KyleSanderson]

Would this be correct ?

is the resulting file generated from the tool? or did you patch it by hand.

[Kenzzer]

Would this be correct ?

is the resulting file generated from the tool? or did you patch it by hand.

It was indeed done by hand. However, now that I try to generate the file I'm getting a lot of compilation errors for the shworker.

fd_hopter.cpp:348:8: error: ‘strncmp’ was not declared in this scope
  348 |   if (!strncmp(buf, "@VARARGS", 8)) {
      |        ^~~~~~~
fd_hopter.cpp:81:1: note: ‘strncmp’ is defined in header ‘<cstring>’; did you forget to ‘#include <cstring>’?

shworker.cpp:171:20: error: ‘strlen’ was not declared in this scope
  171 |  size_t whatsize = strlen(what);
      |                    ^~~~~~
shworker.cpp:53:1: note: ‘strlen’ is defined in header ‘<cstring>’; did you forget to ‘#include <cstring>’?

fd_hopter.cpp:331:16: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
        fprintf(fout, bigblock); 

And finally, the linker freaks out if using gcc, so fixing those compilation errors and using clang does compile the generate bin file.

There's however a last problem, if trying use committed generate.bin file or the one output'd by the makefile gives the following error : Syntax error in expression

I will be investigating further soon.

[Kenzzer]

After a bit of digging, the command needs a parameter of 20 which seems to correspond to the maximum amount of parameters the sourcehook macros will support.

./generate 20

[dvander]

I think this is safe, but it took a fair bit to convince myself.

As a background, the dirty work of hooks is implemented... inside all those DECL macros in plugins. Hook are not implemented or handled in Metamod:Source's core library.

When you declare a hook in a plugin, you're really instantiating a bunch of critical code that implements all of the secret hooking sauce. This code is embedded directly in the plugin. The plugin then gives an opaque pointer to Metamod:Source, which then patches into read-only memory. When the hooked function is invoked, the plugin gets called directly, and then uses a hidden-ish API to ask Metamod:Source for some follow-up information (like "what plugin do I need to call next" and "am I supposed to call the original or stop").

Because the actual invocation work is all done inside plugins, and plugins are talking directly to each other, the macro sauce happens to form an ABI. Adding a virtual dtor breaks ABI, since it tends to re-position everything in the table. This is why we've never added one to IMyDelegate or ISHDelegate.

CMyDelegateImpl would seem to be exempt... except for this line:

    bool IsEqual(ISHDelegate *pOtherDeleg) { return m_Deleg == static_cast<CMyDelegateImpl*>(pOtherDeleg)->m_Deleg; } \

That static_cast means even the local plugin implementation of CMyDelegateImpl must be ABI compatible.

Fortunately, adding a virtual dtor here shouldn't affect the position of m_Deleg, and nothing ever inherits from CMyDelegateImpl. So I think it's safe.