[feature] Issue with Loading Custom Plugins in Endstone due to Plugin Loading Mechanism

[engsr6982]

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I'm trying to write other language engines for Endstone (although the Python functionality provided by Endstone is stronger). However, the plugin loading mechanism of Endstone prevents me from loading plugins of custom types, such as.lua ones.
Because I need to wait for Endstone to load my engine DLL before I can register the loader with Endstone. But by this time, Endstone has already completed the loading and will no longer call the newly added loader.
I've tried to manually call the "loadPlugins" function, but Endstone doesn't check whether the plugins have been loaded, which will lead to an infinite loop. Besides, the members of Endstone_plugin_manager are private, and I can't register them manually either.
Describe the solution you'd like
A clear and concise description of what you want to happen.

Could you add a couple of overloads to EndStonePluginManager::loadPlugins(const std::string &directory), for example:

EndStonePluginManager::loadPlugins(const std::string &directory, string const& loader_type)

or

EndStonePluginManager::loadPlugins(const std::string &directory, PluginLoader& loader)

[wu-vincent]

Thanks for the issue! This is indeed something in the current design that needs improvement.

I’m currently refactoring the plugin loading mechanism to address this. I’ve added a new function, PluginManager::loadPlugin(std::string file), which allows loading a plugin directly from a single file instead of a directory. I’ve also introduced an overload for PluginManager::loadPlugins(std::vector<std::string>) that accepts a list of file paths instead of just a directory.

These changes should enable you to load your custom plugins without encountering infinite loops. Most of the refactoring work mentioned above is completed in this commit, but I still need to perform more testing.

[wu-vincent]

Close as it's completed now. The new API will be available in the upcoming release v0.5.6.

[engsr6982]

Thanks for the issue! This is indeed something in the current design that needs improvement.

I’m currently refactoring the plugin loading mechanism to address this. I’ve added a new function, PluginManager::loadPlugin(std::string file), which allows loading a plugin directly from a single file instead of a directory. I’ve also introduced an overload for PluginManager::loadPlugins(std::vector<std::string>) that accepts a list of file paths instead of just a directory.

These changes should enable you to load your custom plugins without encountering infinite loops. Most of the refactoring work mentioned above is completed in this commit, but I still need to perform more testing.

I tested these two new overloaded functions and found that custom plugins still cannot be properly registered with Endstone.

Here's my registration process:
When Endstone loads my DLL, I register a custom Loader in the onLoad function. Then I search for the file paths I expect to load in the plugins directory and pass them to the PluginManager::loadPlugins(std::vector<std::string> files) function.

However, PluginManager::loadPlugins calls PluginManager::loadPlugin(file), which iterates through each Loader trying to find one that can load this file. The problem occurs here - this function doesn't perform any checks and doesn't call getPluginFileFilters() for filtering. Instead, it directly passes the file path to both Cpp and Py Loaders, causing exceptions and crashes.

[engsr6982]

Thanks for the issue! This is indeed something in the current design that needs improvement.
I’m currently refactoring the plugin loading mechanism to address this. I’ve added a new function, PluginManager::loadPlugin(std::string file), which allows loading a plugin directly from a single file instead of a directory. I’ve also introduced an overload for PluginManager::loadPlugins(std::vector<std::string>) that accepts a list of file paths instead of just a directory.
These changes should enable you to load your custom plugins without encountering infinite loops. Most of the refactoring work mentioned above is completed in this commit, but I still need to perform more testing.

I tested these two new overloaded functions and found that custom plugins still cannot be properly registered with Endstone.

Here's my registration process: When Endstone loads my DLL, I register a custom Loader in the onLoad function. Then I search for the file paths I expect to load in the plugins directory and pass them to the PluginManager::loadPlugins(std::vector<std::string> files) function.

However, PluginManager::loadPlugins calls PluginManager::loadPlugin(file), which iterates through each Loader trying to find one that can load this file. The problem occurs here - this function doesn't perform any checks and doesn't call getPluginFileFilters() for filtering. Instead, it directly passes the file path to both Cpp and Py Loaders, causing exceptions and crashes.

I forked the project and fixed the problem. But I found a more serious problem, if any plugin registers (modifies) the Vector that holds the Loader during onLoad, it will cause the iterator to fail and trigger a crash.

fork: https://github.com/engsr6982/endstone/commit/dd2d97f9f3cd8ee98c5760eca098edb76df322bb
Iterator failure location:

endstone/src/endstone_core/plugin/plugin_manager.cpp

Lines 100 to 119 in 8dd9acf

	std::vector<Plugin *> EndstonePluginManager::loadPlugins(std::string directory)
	{
	std::vector<Plugin *> loaded_plugins;
	// TODO(plugin): handling logic for depend, soft_depend, load_before and provides
	for (const auto &loader : plugin_loaders_) {
	auto plugins = loader->loadPlugins(directory);
	for (const auto &plugin : plugins) {
	if (!plugin) {
	continue;
	}
	if (initPlugin(*plugin, *loader, fs::path(directory))) {
	loaded_plugins.push_back(plugin);
	}
	}
	}
	return loaded_plugins;
	}

[wu-vincent]

To address cases where the vector may be modified during iteration, I’ve made an additional commit to fix the issue. You can check it here: 6ed7faf

Can you verify if this resolves the problem?

[engsr6982]

To address cases where the vector may be modified during iteration, I’ve made an additional commit to fix the issue. You can check it here: 6ed7faf

Can you verify if this resolves the problem?

Thanks, it's been verified and the code is running fine so far!