some more review

This commit is contained in:
Konicai 2024-06-17 00:06:44 -05:00
parent 6159799804
commit 20d91cc201
9 changed files with 29 additions and 23 deletions

View file

@ -166,8 +166,7 @@ public class GeyserBungeePlugin extends Plugin implements GeyserBootstrap {
ExecutionCoordinator.simpleCoordinator(),
sourceConverter
);
this.commandRegistry = new CommandRegistry(geyser, cloud);
this.commandRegistry = new CommandRegistry(geyser, cloud, false); // applying root permission would be a breaking change because we can't register permission defaults
}
// Force-disable query if enabled, or else Geyser won't enable

View file

@ -88,7 +88,7 @@ public class GeyserFabricBootstrap extends GeyserModBootstrap implements ModInit
ExecutionCoordinator.simpleCoordinator(),
sourceConverter
);
this.setCommandRegistry(new CommandRegistry(GeyserImpl.getInstance(), cloud));
this.setCommandRegistry(new CommandRegistry(GeyserImpl.getInstance(), cloud, false)); // applying root permission would be a breaking change because we can't register permission defaults
}
@Override

View file

@ -32,6 +32,8 @@ import org.geysermc.geyser.util.VersionCheckUtils;
public final class GeyserModUpdateListener {
public static void onPlayReady(Player player) {
// Should be creating this in the supplier, but we need it for the permission check.
// Not a big deal currently because ModCommandSource doesn't load locale, so don't need to try to wait for it.
ModCommandSource source = new ModCommandSource(player.createCommandSourceStack());
if (source.hasPermission(Permissions.CHECK_UPDATE)) {
VersionCheckUtils.checkForGeyserUpdate(() -> source);

View file

@ -45,6 +45,7 @@ public class ModCommandSource implements GeyserCommandSource {
public ModCommandSource(CommandSourceStack source) {
this.source = source;
// todo find locale?
}
@Override
@ -96,4 +97,4 @@ public class ModCommandSource implements GeyserCommandSource {
public Object handle() {
return source;
}
}
}

View file

@ -216,7 +216,6 @@ public class GeyserSpigotPlugin extends JavaPlugin implements GeyserBootstrap {
GeyserImpl.start();
if (geyserConfig.isLegacyPingPassthrough()) {
this.geyserSpigotPingPassthrough = GeyserLegacyPingPassthrough.init(geyser);
} else {

View file

@ -61,11 +61,6 @@ public class SpigotCommandRegistry extends CommandRegistry {
this.commandMap = commandMap;
}
@Override
protected boolean applyRootPermission() {
return true; // because we have native permission defaults
}
@NonNull
@Override
public String description(@NonNull String command, @NonNull String locale) {

View file

@ -132,7 +132,7 @@ public class GeyserVelocityPlugin implements GeyserBootstrap {
ExecutionCoordinator.simpleCoordinator(),
sourceConverter
);
this.commandRegistry = new CommandRegistry(geyser, cloud);
this.commandRegistry = new CommandRegistry(geyser, cloud, false); // applying root permission would be a breaking change because we can't register permission defaults
}
GeyserImpl.start();

View file

@ -77,6 +77,7 @@ public class CommandRegistry implements EventRegistrar {
protected final GeyserImpl geyser;
private final CommandManager<GeyserCommandSource> cloud;
private final boolean applyRootPermission;
/**
* Map of Geyser subcommands to their Commands
@ -98,9 +99,28 @@ public class CommandRegistry implements EventRegistrar {
*/
protected final Map<String, TriState> permissionDefaults = new Object2ObjectOpenHashMap<>(13);
/**
* Creates a new CommandRegistry. Does apply a root permission. If undesired, use the other constructor.
*/
public CommandRegistry(GeyserImpl geyser, CommandManager<GeyserCommandSource> cloud) {
this(geyser, cloud, true);
}
/**
* Creates a new CommandRegistry
*
* @param geyser the Geyser instance
* @param cloud the cloud command manager to register commands to
* @param applyRootPermission true if this registry should apply a permission to Geyser and Extension root commands.
* This currently exists because we want to retain the root command permission for Spigot,
* but don't want to add it yet to platforms like Velocity where we cannot natively
* specify a permission default. Doing so will break setups as players would suddenly not
* have the required permission to execute any Geyser commands.
*/
public CommandRegistry(GeyserImpl geyser, CommandManager<GeyserCommandSource> cloud, boolean applyRootPermission) {
this.geyser = geyser;
this.cloud = cloud;
this.applyRootPermission = applyRootPermission;
// register our custom exception handlers
ExceptionHandlers.register(cloud);
@ -225,7 +245,7 @@ public class CommandRegistry implements EventRegistrar {
private void buildRootCommand(String permission, HelpCommand help) {
Builder<GeyserCommandSource> builder = cloud.commandBuilder(help.rootCommand());
if (applyRootPermission()) {
if (applyRootPermission) {
builder = builder.permission(permission);
permissionDefaults.put(permission, TriState.TRUE);
}
@ -241,16 +261,6 @@ public class CommandRegistry implements EventRegistrar {
}));
}
/**
* Returns true if the registry should apply a permission to Geyser and Extension root commands.
* This currently exists because we want to retain the root command permission for Spigot, but don't want to add
* it yet to platforms like Velocity where we cannot natively specify a permission default. Doing so will
* break setups as players would suddenly not have the required permission to execute any Geyser commands.
*/
protected boolean applyRootPermission() {
return false;
}
protected void onRegisterPermissions(GeyserRegisterPermissionsEvent event) {
for (Map.Entry<String, TriState> permission : permissionDefaults.entrySet()) {
event.register(permission.getKey(), permission.getValue());

View file

@ -87,7 +87,7 @@ public record CommandSourceConverter<S>(Class<S> senderType,
public @NonNull S reverse(GeyserCommandSource source) throws IllegalArgumentException {
Object handle = source.handle();
if (senderType.isInstance(handle)) {
return senderType.cast(source); // one of the server platform implementations
return senderType.cast(handle); // one of the server platform implementations
}
if (source.isConsole()) {