Plugin folder vs plugin name

[xoft]

I've been thinking about this and I keep noticing this needs more attention.

Currently the plugins are loaded based on the folder name. This folder name is in turn assigned into the plugin name variable; but that one can be changed by the plugin itself using the cPlugin:SetName() call. I've always wondered why this would be needed at all, and now I have a perfect usecase:

Consider someone downloads a plugin from GitHub using their ZIP download facility. When extracted, this results in a "<RepoName_master>" folder for the plugin. But if that plugin provides any kind of inter-plugin API (accessible via cPluginManager:CallPlugin), the calls won't work because they currently use the plugin's folder to identify the plugin, not the name.

So I have this proposal:
- Use plugin folder to identify plugins to load in the settings.ini
- Each plugin should use the cPlugin:SetName() function to set a well-defined name, if it exposes any functionality as a callable API
- Change MCServer so that it identifies plugin calls by the plugin name, rather than the folder
- Change cPluginManager:GetAllPlugins() binding so that it returns a map of PluginName -> cPlugin, rather than PluginFolder -> cPlugin
- Change Core's Plugin page to handle the changes correctly

What do you think, is this reasonable?

[NiLSPACE]

Makes sense to me. I agree with this

[tigerw]

Use the manifest file to set the name rather than a function call?

[xoft]

That could be done as well, I think I'll keep it for now and perhaps add processing for that later on.

I'm refactoring the cPluginManager, turns out this is a rather large change. Or maybe it's because the class needed refactoring long ago and it was being postponed each time. I guess I'll even break some plugins a little At the very least the Core's plugin webadmin page will need changing, too.

[xoft]

Big changes are finally done.
https://github.com/mc-server/MCServer/pull/1875
https://github.com/mc-server/Core/pull/131

I'd appreciate feedback in all the facets: cPluginManager internals, the API, and the Core's console and WebAdmin changes.

[jan64]

Given that this removes GetPlugin, how are plugins supposed to check version/existence of another plugin they are trying to hook into?

[NiLSPACE]

With cPluginManager::DoWithPlugin(Plugin, Callback)

[jan64]

	if cPluginManager:Get():IsPluginLoaded("Core") then
		cPluginManager:DoWithPlugin("Core",
			function (PluginHandle)
				if PluginHandle:GetVersion() >= 15 then
					if cPluginManager:CallPlugin("Core", "AddWebChatCallback", "IRChat", "OnWebChat") == true then
						HookedIntoCore = true
						LOG("[IRChat] Hooked into Core")
					else 
						HookingError("Core", 1, "CallPlugin didn't return true", "Web endpoint will be unavialable")
					end
				else
					HookingError("Core", 2, "Your Core is outdated, the minimum version is 15", "Web endpoint will be unavialable")
				end
			end
		)
	else
		HookingError("Core", 3, "Core not found", "Web endpoint will be unavialable")
	end

This doesn't really work. (I'm using for each plugin as a workaround atm)

Code:

[13:08:24] LUA: Plugins/IRChat/Hooks.lua:10: attempt to call method 'DoWithPlugin' (a nil value)
[13:08:24] Stack trace:
[13:08:24]   Plugins/IRChat/Hooks.lua(10): (no name)
[13:08:24] Stack trace end
[13:08:24] Error in plugin IRChat calling function <callback>()

[NiLSPACE]

Hmm, it might not be exported yet.

[xoft]

I'm exporting it now.

EDIT: And fixing the ForEachPlugin so that it is a static function.