Reloading with PlugManX doesn't work at all

[xLexih]

CommandAPI version

9.3.0

Minecraft version

1.20.1

Are you shading the CommandAPI?

Yes

What I did

I setup a 1.20.2 paper server with PlugmanX and my own plugin with the following:

override fun onLoad() {
    instance = this
    CommandAPI.onLoad(CommandAPIBukkitConfig(this)
    .missingExecutorImplementationMessage("You cannot execute this command!")
    .dispatcherFile(File(dataFolder, "command_registration.json")));
}
 override fun onEnable() {
    CommandAPI.onEnable()
    CommandAPICommand("testreregistration")
      .withArguments(StringArgument("testString"))
      .withArguments(LiteralArgument.of("success"))
      .executes(CommandExecutor { executor, args ->
        val testString: String = args.get("testString") as String
        executor.sendMessage(Component.text("The test String was: '" + testString + "'"));
      }).register();
}
override fun onDisable() {
    CommandAPI.unregister("testreregistration", true)
    CommandAPI.onDisable()
}

server starts up normally but using /plugman reload <plugin> causes the issue

What actually happened

When the server starts up normally the command works fine, but once you run /plugman reload <plugin> the plugin reloads and if the command was unregistered in onDisable() the command disappears and never gets registered again.

What should have happened

It should unregister and register the command similar to how /reload confirm does it

Server logs and CommandAPI config

No response

Other

No response

[DerEchtePilz]

I've been digging around in PlugManX's source and it should be totally their fault. They assume that every command is added by a plugin as far as my quick analysis goes. With added by a plugin, I mean by using the well known method that Bukkit added and not using a command framework. The commands registered by the CommandAPI do not pass a check if they are PluginCommands since we do not use the BukkitPluginCommand class. I believe this issue should also be opened on their GitHub which you linked to make them aware of commands that do not have plugins associated with them.

The main issue seems to be their BukkitCommandWrap class which seems to only load commands that are wrapped with a BukkitCommandWrapper. The CommandAPI, however, uses the VanillaCommandWrapper class and from my quick analysis, PlugManX does not reload these commands. It is possible that I have missed something, but opening this on their GitHub too would certainly not be a bad idea.

[Test-Account666]

I've been digging around in PlugManX's source and it should be totally their fault. They assume that every command is added by a plugin as far as my quick analysis goes. With added by a plugin, I mean by using the well known method that Bukkit added and not using a command framework. The commands registered by the CommandAPI do not pass a check if they are PluginCommands since we do not use the BukkitPluginCommand class. I believe this issue should also be opened on their GitHub which you linked to make them aware of commands that do not have plugins associated with them.

The main issue seems to be their BukkitCommandWrap class which seems to only load commands that are wrapped with a BukkitCommandWrapper. The CommandAPI, however, uses the VanillaCommandWrapper class and from my quick analysis, PlugManX does not reload these commands. It is possible that I have missed something, but opening this on their GitHub too would certainly not be a bad idea.

Well, as long as my BukkitCommandWrapper doesn't somehow delete the command, there shouldn't be any issues actually registering the command.

Commands using VanillaCommandWrapper should also get properly loaded since I'm also calling CraftServer#syncCommands shortly after.

(Which probably makes the rest of my BukkitCommandWrapper setup useless)

[DerEchtePilz]

@TheBlackEntity Whether it's your command wrapper class or not, the plugin causing this issue definitely is your plugin since reloading using the built-in /reload command works without issues (well, if you're not considering it's already flawed nature that is). I can probably spend some time tomorrow and investigate this issue further and see what causes what. I can also probably try some more scenarios and see if other setups work while others don't. However, right now I am fairly confident that your plugin causes this issue.

[DerEchtePilz]

@TheBlackEntity For what it's worth, I did experiment a little yesterday and could only confirm that it is your plugin that causes the issue. I've also cloned your repo and put a few debug statements that should be run when reloading a plugin on a Paper server. This was the log I got:

[19:24:38 INFO]: DerEchtePilz issued server command: /testreregistration test success
[19:24:46 INFO]: DerEchtePilz issued server command: /plugman reload BugTestingCommandAPI
[19:24:46 INFO]: [PlugManX] [STDOUT] Reload started!
[19:24:46 INFO]: [PlugManX] [STDOUT] Using PaperPluginUtil to reload!
[19:24:46 INFO]: [PlugManX] [STDOUT] Unloading plugin. Command wrapper: BukkitCommandWrap
[19:24:46 INFO]: [PlugManX] [STDOUT] Unloading commands...
[19:24:46 INFO]: [PlugManX] [STDOUT] The size of the internal command map: 0
[19:24:46 INFO]: [BugTestingCommandAPI] Disabling BugTestingCommandAPI v0.0.1
[19:24:47 INFO]: [BugTestingCommandAPI] Loading server plugin BugTestingCommandAPI v0.0.1
[19:24:47 INFO]: [CommandAPI] Loaded platform NMS_1_20_R3 > NMS_Common > CommandAPIBukkit
[19:24:47 INFO]: [CommandAPI] Hooked into Spigot successfully for Chat/ChatComponents
[19:24:47 INFO]: [CommandAPI] Hooked into Adventure for AdventureChat/AdventureChatComponents
[19:24:47 INFO]: [CommandAPI] Hooked into Paper for paper-specific API implementations
[19:24:47 INFO]: [BugTestingCommandAPI] Enabling BugTestingCommandAPI v0.0.1
[19:24:47 INFO]: [CommandAPI] Hooked into Paper ServerResourcesReloadedEvent
[19:24:47 INFO]: [CommandAPI] Linked 2 Bukkit permissions to commands
[19:24:47 INFO]: [CommandAPI] Reloading datapacks...
[19:24:47 INFO]: Loaded 1174 recipes
[19:24:47 INFO]: Loaded 1271 advancements
[19:24:48 INFO]: [CommandAPI] Finished reloading datapacks

Now, you will probably have noticed this line:

[19:24:46 INFO]: [PlugManX] [STDOUT] The size of the internal command map: 0

This was put in BukkitPluginUtil#unloadCommands(Plugin plugin) on line 713

I am now really unsure about the actual command reloading process given that this map was empty.

I also did some other tests that shouldn't really fail as the CommandAPI was handling that but I didn't want to miss anything. I registered this command:

new CommandAPICommand("lookupcommand")
    .withArguments(new StringArgument("commands")
        .replaceSuggestions(ArgumentSuggestions.stringsAsync(info ->
                CompletableFuture.supplyAsync(() -> CommandAPI.getRegisteredCommands().stream().map(RegisteredCommand::commandName).toArray(String[]::new))
            )
        )
    )
    .executesPlayer(info -> {
        Player player = info.sender();
        String command = (String) info.args().get("commands");
        Map<String, Command> knownCommands = Bukkit.getCommandMap().getKnownCommands();
        if (knownCommands.containsKey(command)) {
            player.sendMessage(Component.text("The command '" + command + "' was found in the command map!"));
            player.sendMessage(Component.text("The command is of type: " + knownCommands.get(command).getClass().getSimpleName()));
        }
        CommandNode<?> commandNode = Brigadier.getCommandDispatcher().getRoot().getChild(command);
        if (commandNode != null) {
            player.sendMessage(Component.text("Command is present in the CommandAPI's Brigadier dispatcher"));
            player.sendMessage(Component.text("The CommandNode is of type: " + commandNode.getClass().getSimpleName()));
        }
    })
    .register();

As expected both if-statements pass and these classes are, again as expected, VanillaCommandWrapper and LiteralCommandNode.

However, I noticed something weird today. While testing yesterday, I could reproduce this issue with no problems whatsoever. Today, I couldn't. I changed nothing and only added one more command (which shouldn't affect anything, really) and /plugman reload TestPlugin worked somewhat. I say somewhat because suggestions definitely break under some circumstances but that's just normal reload shenanigans.

In the end, I can say that this issue is really weird. Yesterday I could reproduce it, today I can't. I don't know what causes this to suddenly work, especially because it wasn't only me who encountered this issue obviously.

[Test-Account666]

@TheBlackEntity For what it's worth, I did experiment a little yesterday and could only confirm that it is your plugin that causes the issue. I've also cloned your repo and put a few debug statements that should be run when reloading a plugin on a Paper server. This was the log I got:

[19:24:38 INFO]: DerEchtePilz issued server command: /testreregistration test success
[19:24:46 INFO]: DerEchtePilz issued server command: /plugman reload BugTestingCommandAPI
[19:24:46 INFO]: [PlugManX] [STDOUT] Reload started!
[19:24:46 INFO]: [PlugManX] [STDOUT] Using PaperPluginUtil to reload!
[19:24:46 INFO]: [PlugManX] [STDOUT] Unloading plugin. Command wrapper: BukkitCommandWrap
[19:24:46 INFO]: [PlugManX] [STDOUT] Unloading commands...
[19:24:46 INFO]: [PlugManX] [STDOUT] The size of the internal command map: 0
[19:24:46 INFO]: [BugTestingCommandAPI] Disabling BugTestingCommandAPI v0.0.1
[19:24:47 INFO]: [BugTestingCommandAPI] Loading server plugin BugTestingCommandAPI v0.0.1
[19:24:47 INFO]: [CommandAPI] Loaded platform NMS_1_20_R3 > NMS_Common > CommandAPIBukkit
[19:24:47 INFO]: [CommandAPI] Hooked into Spigot successfully for Chat/ChatComponents
[19:24:47 INFO]: [CommandAPI] Hooked into Adventure for AdventureChat/AdventureChatComponents
[19:24:47 INFO]: [CommandAPI] Hooked into Paper for paper-specific API implementations
[19:24:47 INFO]: [BugTestingCommandAPI] Enabling BugTestingCommandAPI v0.0.1
[19:24:47 INFO]: [CommandAPI] Hooked into Paper ServerResourcesReloadedEvent
[19:24:47 INFO]: [CommandAPI] Linked 2 Bukkit permissions to commands
[19:24:47 INFO]: [CommandAPI] Reloading datapacks...
[19:24:47 INFO]: Loaded 1174 recipes
[19:24:47 INFO]: Loaded 1271 advancements
[19:24:48 INFO]: [CommandAPI] Finished reloading datapacks

Now, you will probably have noticed this line:

[19:24:46 INFO]: [PlugManX] [STDOUT] The size of the internal command map: 0

This was put in BukkitPluginUtil#unloadCommands(Plugin plugin) on line 713

I am now really unsure about the actual command reloading process given that this map was empty.

I also did some other tests that shouldn't really fail as the CommandAPI was handling that but I didn't want to miss anything. I registered this command:

new CommandAPICommand("lookupcommand")
    .withArguments(new StringArgument("commands")
        .replaceSuggestions(ArgumentSuggestions.stringsAsync(info ->
                CompletableFuture.supplyAsync(() -> CommandAPI.getRegisteredCommands().stream().map(RegisteredCommand::commandName).toArray(String[]::new))
            )
        )
    )
    .executesPlayer(info -> {
        Player player = info.sender();
        String command = (String) info.args().get("commands");
        Map<String, Command> knownCommands = Bukkit.getCommandMap().getKnownCommands();
        if (knownCommands.containsKey(command)) {
            player.sendMessage(Component.text("The command '" + command + "' was found in the command map!"));
            player.sendMessage(Component.text("The command is of type: " + knownCommands.get(command).getClass().getSimpleName()));
        }
        CommandNode<?> commandNode = Brigadier.getCommandDispatcher().getRoot().getChild(command);
        if (commandNode != null) {
            player.sendMessage(Component.text("Command is present in the CommandAPI's Brigadier dispatcher"));
            player.sendMessage(Component.text("The CommandNode is of type: " + commandNode.getClass().getSimpleName()));
        }
    })
    .register();

As expected both if-statements pass and these classes are, again as expected, VanillaCommandWrapper and LiteralCommandNode.

However, I noticed something weird today. While testing yesterday, I could reproduce this issue with no problems whatsoever. Today, I couldn't. I changed nothing and only added one more command (which shouldn't affect anything, really) and /plugman reload TestPlugin worked somewhat. I say somewhat because suggestions definitely break under some circumstances but that's just normal reload shenanigans.

In the end, I can say that this issue is really weird. Yesterday I could reproduce it, today I can't. I don't know what causes this to suddenly work, especially because it wasn't only me who encountered this issue obviously.

Thank you for taking some time to test this.

A weird issue indeed.

I'll see what I can do to add support for CommandAPI.

I'm also unsure what's going on with the command map in unloadCommands...

[DerEchtePilz]

I'm also unsure what's going on with the command map in unloadCommands...

Just so you know, I was talking about the commands field.

[DerEchtePilz]

I think I am gonna close this issue as this is nothing caused by the CommandAPI but rather is caused by the already flawed concept of reloading plugins at runtime.