register all permissions on NeoForge

bootstrap/mod/neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgeCommandRegistry.java

@@ -26,20 +26,65 @@
 package org.geysermc.geyser.platform.neoforge;
 
 import org.geysermc.geyser.GeyserImpl;
+import org.geysermc.geyser.api.event.lifecycle.GeyserRegisterPermissionsEvent;
+import org.geysermc.geyser.api.util.TriState;
 import org.geysermc.geyser.command.CommandRegistry;
+import org.geysermc.geyser.command.GeyserCommand;
 import org.geysermc.geyser.command.GeyserCommandSource;
 import org.incendo.cloud.CommandManager;
 import org.incendo.cloud.neoforge.PermissionNotRegisteredException;
 
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 public class GeyserNeoForgeCommandRegistry extends CommandRegistry {
+
+    /**
+     * Permissions with an undefined permission default. Use Set to not register the same fallback more than once.
+     */
+    private final Set<String> undefinedPermissions = new HashSet<>();
+
     public GeyserNeoForgeCommandRegistry(GeyserImpl geyser, CommandManager<GeyserCommandSource> cloud) {
         super(geyser, cloud);
     }
 
+    @Override
+    protected void register(GeyserCommand command, Map<String, GeyserCommand> commands) {
+        super.register(command, commands);
+
+        if (!command.permission().isBlank() && command.permissionDefault() == null) {
+            // Permission requirement exists but no default value specified.
+
+            // Generally, we don't register a permission if no default is specified.
+            // However, NeoForge requires that all permissions are registered, and cloud-neoforge follows that.
+            undefinedPermissions.add(command.permission());
+        }
+    }
+
+    @Override
+    protected void onRegisterPermissions(GeyserRegisterPermissionsEvent event) {
+        super.onRegisterPermissions(event);
+
+        // Now that we are aware of all commands, we can determine which ones are actually undefined
+        // (two commands may have the same permission, but only of them defines a permission default).
+        // Note: This shouldn't be that necessary, as GeyserNeoForgePermissionHandler will ignore
+        // anything already registered. Trying to rely on that as little as possible though.
+        undefinedPermissions.removeAll(permissionDefaults.keySet());
+
+        // Register with NOT_SET as a fallback.
+        // If extensions wish, they may register permissions in an earlier listener, which won't be overridden.
+        for (String permission : undefinedPermissions) {
+            geyser.getLogger().debug("Registering permission " + permission + " with fallback default value of NOT_SET");
+            event.register(permission, TriState.NOT_SET);
+        }
+    }
+
     @Override
     public boolean hasPermission(GeyserCommandSource source, String permission) {
         // NeoForgeServerCommandManager will throw this exception if the permission is not registered to the server.
         // We can't realistically ensure that every permission is registered (calls by API users), so we catch this.
+        // This works for our calls, but not for cloud's internal usage. For that case, see above.
         try {
             return super.hasPermission(source, permission);
         } catch (PermissionNotRegisteredException e) {

bootstrap/mod/neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgePermissionHandler.java

@@ -46,12 +46,11 @@ public class GeyserNeoForgePermissionHandler {
         );
     }
 
-    private void registerNode(String node, TriState permissionDefault, PermissionGatherEvent.Nodes event) {
+    private void registerNode(String permission, TriState permissionDefault, PermissionGatherEvent.Nodes event) {
-        PermissionNode<Boolean> permissionNode = createNode(node, permissionDefault);
-
         // NeoForge likes to crash if you try and register a duplicate node
-        if (event.getNodes().stream().noneMatch(eventNode -> eventNode.getNodeName().equals(node))) {
+        if (event.getNodes().stream().noneMatch(n -> n.getNodeName().equals(permission))) {
-            event.addNodes(permissionNode);
+            PermissionNode<Boolean> node = createNode(permission, permissionDefault);
+            event.addNodes(node);
         }
     }
 

core/src/main/java/org/geysermc/geyser/command/CommandRegistry.java

@@ -27,6 +27,7 @@ package org.geysermc.geyser.command;
 
 import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.geysermc.event.PostOrder;
 import org.geysermc.geyser.GeyserImpl;
 import org.geysermc.geyser.api.command.Command;
 import org.geysermc.geyser.api.event.EventRegistrar;
@@ -74,7 +75,7 @@ public class CommandRegistry implements EventRegistrar {
 
     private static final String GEYSER_ROOT_PERMISSION = "geyser.command";
 
-    private final GeyserImpl geyser;
+    protected final GeyserImpl geyser;
     private final CommandManager<GeyserCommandSource> cloud;
 
     /**
@@ -95,7 +96,7 @@ public class CommandRegistry implements EventRegistrar {
     /**
      * Map containing only permissions that have been registered with a default value
      */
-    private final Map<String, TriState> permissionDefaults = new Object2ObjectOpenHashMap<>(13);
+    protected final Map<String, TriState> permissionDefaults = new Object2ObjectOpenHashMap<>(13);
 
     public CommandRegistry(GeyserImpl geyser, CommandManager<GeyserCommandSource> cloud) {
         this.geyser = geyser;
@@ -159,8 +160,9 @@ public class CommandRegistry implements EventRegistrar {
             buildRootCommand("geyser.extension." + id + ".command", extensionHelp);
         }
 
-        // wait for the right moment (depends on the platform) to register permissions
+        // Wait for the right moment (depends on the platform) to register permissions.
-        geyser.eventBus().subscribe(this, GeyserRegisterPermissionsEvent.class, this::onRegisterPermissions);
+        // Listen late so that extensions can register permissions before this class does
+        geyser.eventBus().subscribe(this, GeyserRegisterPermissionsEvent.class, this::onRegisterPermissions, PostOrder.LATE);
     }
 
     /**
@@ -182,7 +184,7 @@ public class CommandRegistry implements EventRegistrar {
         register(command, this.extensionCommands.computeIfAbsent(extension, e -> new HashMap<>()));
     }
 
-    private void register(GeyserCommand command, Map<String, GeyserCommand> commands) {
+    protected void register(GeyserCommand command, Map<String, GeyserCommand> commands) {
         command.register(cloud);
 
         commands.put(command.name(), command);
@@ -192,8 +194,17 @@ public class CommandRegistry implements EventRegistrar {
             commands.put(alias, command);
         }
 
-        if (!command.permission().isBlank() && command.permissionDefault() != null) {
+        String permission = command.permission();
-            permissionDefaults.put(command.permission(), command.permissionDefault());
+        TriState defaultValue = command.permissionDefault();
+        if (!permission.isBlank() && defaultValue != null) {
+
+            TriState existingDefault = permissionDefaults.get(permission);
+            // Extensions might be using the same permission for two different commands
+            if (existingDefault != null && existingDefault != defaultValue) {
+                geyser.getLogger().debug("Overriding permission default %s:%s with %s".formatted(permission, existingDefault, defaultValue));
+            }
+
+            permissionDefaults.put(permission, defaultValue);
         }
     }
 
@@ -234,7 +245,7 @@ public class CommandRegistry implements EventRegistrar {
         return false;
     }
 
-    private void onRegisterPermissions(GeyserRegisterPermissionsEvent event) {
+    protected void onRegisterPermissions(GeyserRegisterPermissionsEvent event) {
         for (Map.Entry<String, TriState> permission : permissionDefaults.entrySet()) {
             event.register(permission.getKey(), permission.getValue());
         }