alliedmodders / sourcemod

SourceMod - Source Engine Scripting and Administration
http://www.sourcemod.net/
965 stars 422 forks source link

Extensions don't trigger sourcepawn OnLibraryAdded unless you reload them #2170

Open bottiger1 opened 1 month ago

bottiger1 commented 1 month ago

Extensions that call sharesys->RegisterLibrary don't trigger OnLibraryAdded unless they are reloaded.

SourceMod Version Information: SourceMod Version: 1.12.0.7140 SourcePawn Engine: 1.12.0.7140, interp-x86 (build 1.12.0.7140 NO JIT) SourcePawn API: v1 = 5, v2 = 16 Compiled on: May 21 2024 12:31:10 Built from: https://github.com/alliedmodders/sourcemod/commit/0f588b87 Build ID: 7140:0f588b87 http://www.sourcemod.net/

Example


public Extension __ext_testlib = 
{
    name = "testlib",
    file = "testlib.ext",
    autoload = 1,
    required = 0,
};

native void TestNative();
public void __ext_testlib_SetNTVOptional()
{
    MarkNativeAsOptional("TestNative");
}

public void OnLibraryAdded(const char[] name)
{
  PrintToServer("OnLibraryAdded %s", name); // doesn't show testlib unless you reload the extension
}

public void OnAllPluginsLoaded()
{
  PrintToServer("loaded: %i", LibraryExists("testlib")); // always works
}

void Sample::SDK_OnAllLoaded()
{
    sharesys->RegisterLibrary(myself, "testlib"); // doesn't matter if you put this here or on SDK_OnLoad
    sharesys->AddNatives(myself, g_natives);
}
Kenzzer commented 1 month ago

Is this a bug report or a feature request ? Is the behaviour you're describing of the forward different if you were to have used a plugin library as dependency instead ?

MAGNAT2645 commented 1 month ago

I assume this is a bug report because I also faced this problem. I wanted to use SteamWorks as an optional feature in my plugin so I added #undef REQUIRE_EXTENSIONS before including SteamWorks and added global boolean to track its state (so if it's true I can call its natives). But SteamWorks does not call OnLibraryAdded on first launch.

So in addition to OnLibraryAdded I had to add LibraryExists check in OnAllPluginsLoaded because at this point all extensions are loaded.

Kenzzer commented 1 month ago

Right, but this might just be a load order issue, rather than a bug. If plugin A has X extension as dependency, and plugin B has X extension optional dependency. If plugin A loads before plugin B, then X extension has already been loaded, and OnLibraryAdded will not fire for subsequent loaded plugins. Hence why I'm asking if that behaviour is consistent with plugins that have other plugin library as dependencies.

MAGNAT2645 commented 1 month ago

Yeah, but should this happen on first server start (or restart) when all plugins are loading at the same time, not late ones?

bottiger1 commented 1 month ago

Right, but this might just be a load order issue, rather than a bug. If plugin A has X extension as dependency, and plugin B has X extension optional dependency. If plugin A loads before plugin B, then X extension has already been loaded, and OnLibraryAdded will not fire for subsequent loaded plugins. Hence why I'm asking if that behaviour is consistent with plugins that have other plugin library as dependencies.

This isn't a load order issue.

OnLibraryAdded is supposed to fire after everything is loaded to send plugins a complete list of libraries. It is not supposed to activate the very instant a library is loaded and give you an incomplete list of libraries based on the random order your plugin was loaded.

https://github.com/alliedmodders/sourcemod/blob/4e73713fabedaa39848b7b4f85700990d519b55f/core/sourcemod.cpp#L510

https://github.com/alliedmodders/sourcemod/blob/4e73713fabedaa39848b7b4f85700990d519b55f/core/logic/PluginSys.cpp#L858

https://github.com/alliedmodders/sourcemod/blob/4e73713fabedaa39848b7b4f85700990d519b55f/core/logic/PluginSys.cpp#L1069

https://github.com/alliedmodders/sourcemod/blob/4e73713fabedaa39848b7b4f85700990d519b55f/core/logic/PluginSys.cpp#L1386

Not to mention that I don't have any other plugin including this extension I tested on, and it doesn't matter if I put RegisterLibrary in SDK_OnLoad instead of SDK_OnAllLoaded. The behavior is the same.

bottiger1 commented 1 month ago

I'm guessing you just need to add another line of code under the last one to loop through all existing extensions (m_Lib in the extensionmanager)

Then for each extension you loop through m_Libraries, and call pPlugin->Call_OnLibraryAdded(library);

I have no idea why it works when the extension is reloaded though. I don't really want to spend many more hours staring at the sourcemod internals given that I have a lot of other stuff to do still.

KyleSanderson commented 1 month ago

This is intended, but I do not agree with it. The calling pattern is you call LibraryExists or whatever the native is when your plugin loads.

bottiger1 commented 1 month ago

This is intended, but I do not agree with it. The calling pattern is you call LibraryExists or whatever the native is when your plugin loads.

I really don't see how this can be intended. What benefit is there for it to act this way? Why are plugin based libraries handled correctly then but not extensions?

It looks more like an oversight to me where plugin logic is there but extension logic is missing like this https://github.com/alliedmodders/sourcemod/issues/2169

In a correctly working function like LibraryExists, it calls both the plugin system and the extension system.

    if (scripts->LibraryExists(str))
    {
        return 1;
    }

    if (extsys->LibraryExists(str))
    {
        return 1;
    }

So to me it looks very wrong here when only the plugin system is called but not the extension system. Where is the equivalent of ForEachLibrary on ExtensionSys?

        pl->ForEachLibrary([pPlugin] (const char *lib) -> void {
            pPlugin->Call_OnLibraryAdded(lib);
        });

This seems like a relatively easy fix here, but I'm just going to use LibraryExists for now as I still have a lot of other 64bit work to do that takes priority.