Open SOF3 opened 6 years ago
dktapps
commented
6 years ago This is a problem that does not actually exist since you cannot load two plugins with the same name regardless.
It's also a bad idea to allow this, because there would be issues like, if multiple plugins share the same name, they'll encounter problems like data directory collisions, fetching the plugin from the PluginManager would have unexpected results, etc.
I don't think the costs of this are worth the gains.
SOF3
commented
6 years ago The problem is exactly about not being able to load two plugins with the same name (not same namespace).
I am not sure what you mean about data dictionary collisions, but plugins should identify all their data using an actually unique identifier. Right now they only identify by plugin name (data folders, NBT tags, MySQL schema names, etc.) (MySQL is a marginal case here because it can be configured by the user). This should get changed according to this issue.
About collisions of fetching plugin from PluginManager, as I mentioned above, plugins can be identified by the main class name rather than plugin name. This is more intuitive for fetching plugins, e.g. If you call getPlugin(DevTools::class) it returns an instance of DevTools. It is not reasonable to disallow users from using two plugins just because they have the same name, even if they are irrelevant (suppose someone makes a plugin that spawns sheep and clashes with @KnownUnown's Sheep plugin).
This is indeed a significant BC break, but I think we should clearly list out all potential benefits and drawbacks before determining whether the costs outweigh the gain. Particularly, the increasing complexity of the plugin ecosystem should be taken into account. Our mechanism works in the old days when everyone downloaded plugins from the same source (http://forums.pocketmine.net/plugins), but this is no longer the case in the past 3 years. Diverged plugin source makes it harder to prevent name clashes, so I believe it is increasingly necessary to enforce plugin identifier qualification.
dktapps
commented
6 years ago I am not sure what you mean about data dictionary collisions,
Data directory collisions. Plugin data has been stored in a folder following the plugin name since time immemorial. Changing this will require BC hacks that would have to remain in the core forever. This applies to pretty much everything else you described as well, except on the plugins themselves.
As I said, I don't think the benefits justify the costs.
dktapps
commented
5 years ago With regards to the getPlugin() suggestion: I think this change could be useful on its own without extra changes. The primary reason being that it is more IDE-friendly than a hardcoded string.
Introduction
Currently, plugins are uniquely identified by their names. This is not a good solution, because plugin names tend to be short and easily clash. A more detailed unique identifier for plugins would be desirable.
Review: What is everyone else doing?
OS level
Windows:
Program Files/Program Files (x86)folders. Similar grouping as Start Menu.Android:
net.pocketmine.server.Language level
Java (maven): each artifact has a group ID and an artifact ID. Apart from the earliest Apache libraries, most libraries are in the format
package.name:artifact-name. JavaScript (npm): packages are identified by name, but they are moving towards namespaced identifiers too. PHP (composer/packagist): packages are identified by GitHub username + GitHub repo name. Note that this forces the use of GitHub, and for whatever political or psychological reasons, some people do not like using services under Microsoft.Comparison
Most PocketMine plugins can be found on GitHub, so an approach similar to composer's could be appropriate. Since the same user is unlikely to have multiple plugins with the same name (*cough* @TheFixerDevelopment), author name + plugin name could be sufficient. However, it is quite common to see multiple plugins in the same repo. This might lead to some inconsistency.
Reference data from Poggit
Number of repos that have `n` plugin/virion projects in the same repo ``` +----+------+ | n | freq | +----+------+ | 1 | 2203 | | 2 | 98 | | 3 | 13 | | 4 | 13 | | 5 | 2 | | 6 | 6 | | 9 | 2 | | 10 | 2 | | 11 | 1 | | 12 | 8 | | 15 | 1 | | 16 | 1 | | 26 | 1 | | 32 | 1 | | 35 | 1 | | 41 | 4 | | 44 | 2 | | 45 | 1 | | 48 | 1 | +----+------+ ``` Number of projects that don't have plugin.yml in the repo root directory: 895/3150 = 28.41%Since PocketMine's developer community is relatively small, there is no need to worry about author name clashes. (There aren't many people with a short name like
SOFe) As a result, there is no need to require a domain name (e.g.io.github.pmmp,com.gmail.sofe2038, etc.).Backward compatibility
plugin.yml needs to be updated
The author fields in plugin.yml are
[0, inf), i.e. we may be unable to determine the author name, or we may have too many author names to use. Therefore, it is not a good solution to useauthor/authorsin plugin.yml to detect the author name.A potential solution is to use the
mainor its namespace as the identifier. This is convenient for developers to request dependencies too, because they just have to writePluginManager->getPlugin(AnotherPluginMain::class)to request an instance. A bonus is that namespaces must not clash anyway due to PHP's constraints.PluginManager::getPlugin() incompatibility
getPlugin()uses the plugin name as the unique identifier. This is no longer possible if the plugin name is no longer the unique identifier. Hence, all plugins that use this method will break.Developers' motivation
At Poggit, I have been requiring developers to use unique namespaces. However, we still occasionally receive plugins that do not conform to these rules. PQRS's rules are also less strict than required in this issue.