Skip to content

Commit

Permalink
Executor rewrite
Browse files Browse the repository at this point in the history 
Note: This commit makes this branch backwards incompatible, especially for non-DSL Kotlin code (see `commandapi-documentation-code/.../Examples.kt`). "Standard" Java and DSL API code usage looks the same, but I'm pretty sure plugins will need to recompile due to changes to the `FunctionalInterface`s. There are also some smaller public API changes, mentioned below.

Notable changes:
- Removed `AbstractCommandSender` and all its implemenations (i.e. the `dev.jorel.commandapi.commandsenders` package is completely gone)
  - Logic in `CommandAPIHandler#generateBrigadierRequirements` for checking if a sender satisfies a `CommandPermission` was moved to `CommandAPIPlatform#getPermissionCheck`
    - Previously, methods in `AbstractCommandSender` provided access to `hasPermission` and `isOp`. `CommandAPIBukkit` and `CommandAPIVelocity` now handle these definitions.
    -  `CommandPermission.TRUE()` and `CommandPermission.FALSE()` added for computing short circuits when combining permissions and requirements
  - `PreviewInfo` now has the generic parameter `Player` rather than an `AbstractPlayer`
    - Backwards-incompatible (`(Player) player.getSource()` becomes `player`)
    - Generic parameter propogates to `PreviewableFunction` and `Previewable`
  - `CommandAPIPlatform` methods for convert Brigadier Source to CommandSender simplified
    - `getSenderForCommand` removed
      - just pass `CommandContext#getSource` into `getCommandSenderFromCommandSource`
      - `forceNative` parameter was only used on Bukkit, which is now handled by `NMS#getNativeProxyCommandSender` (more on that below)
    - `wrapCommandSender` removed
    - Wrapping the sender no longer necessary for `getBrigadierSourceFromCommandSender`
    - `CommandAPIVelocity` now does nothing in these methods :P

- `CommandAPIExecutor` reworked
  - `ExecutorType` moved to platform modules (so Velocity can no longer try to define sender types that don't exist)
  - Detecting whether a certain executor can be run by the given class delegated to `BukkitTypedExecutor` and `VelocityTypedExecutor`
  - Priority of executors now depends on order of calling the `executes` methods (i.e the priority listed here https://commandapi.jorel.dev/9.4.1/normalexecutors.html#multiple-command-executor-implementations no longer applies)
  - Normal executors are no longer ignored if resulting executors exist (not sure if this was a bug or intended)

- Tweaked `ExecutionInfo`
  - Added `CommandContext<Source> cmdCtx` to `ExecutionInfo`
    - This allows passing the information needed for a `NATIVE` executor on Bukkit to create a `NativeProxyCommandSender`
      - Uses `NMS#getNativeProxyCommandSender`, which was adapted from the removed `getSenderForCommand` method
      - Note: conflicts with #478, though should be easily resolved
    - Note: Velocity can define the `CommandSource` object, though Bukkit has it as `?`. This makes it hard for non DSL Kotlin to infer the type parameters for some reason, which is annoying because you now need to specify the type parameter. I don't know if there's a better way to handle that.
    - TODO: Make sure depdendents can still use `ExecutionInfo` without including Brigadier as a dependency
  - `ExecutionInfo` is now a record (`BukkitExecutionInfo` and `VelocityExecutionInfo` removed)

- Simplified `dev.jorel.commandapi.executors` package
  - Platform and sender specific executor classes (e.g. `PlayerCommandExecutor` and `EntityResultingExecutionInfo`) removed
  - Just the functional interfaces `NormalExecutorInfo`, `NormalExecutor`, `ResultingExecutorInfo`, and `ResultingExecutor` now
  - `BukkitExecutable` and `VelocityExecutable` link different command sender classes as type parameters to the `ExecutorType` enum values

TODO:
- Add executor tests to `dev/dev` to ensure same behavior
  - Especially for `PROXY` and `NATIVE` senders
  - Especially for non-DSL Kotlin to see how its lamdba type inference works
- Update documentation
  • Loading branch information
@willkroboth
willkroboth committed May 17, 2024
1 parent 804e68b commit 3077c34
Show file tree
Hide file tree
Showing 148 changed files with 1,333 additions and 3,924 deletions.
15 changes: 6 additions & 9 deletions commandapi-core/src/main/java/dev/jorel/commandapi/Brigadier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import dev.jorel.commandapi.arguments.AbstractArgument;
import dev.jorel.commandapi.arguments.ArgumentSuggestions;
import dev.jorel.commandapi.arguments.Literal;
import dev.jorel.commandapi.commandsenders.AbstractCommandSender;

import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -206,19 +205,17 @@ Command fromCommand(AbstractCommandAPICommand<?, Argument, CommandSender> comman
*/
public static <CommandSender> Object getBrigadierSourceFromCommandSender(CommandSender sender) {
CommandAPIPlatform<?, CommandSender, ?> platform = (CommandAPIPlatform<?, CommandSender, ?>) CommandAPIHandler.getInstance().getPlatform();
return platform.getBrigadierSourceFromCommandSender(platform.wrapCommandSender(sender));
return platform.getBrigadierSourceFromCommandSender(sender);
}

/**
* Returns a Bukkit CommandSender from a Brigadier CommandContext
* Returns the current platform's command sender from a Brigadier CommandContext
*
* @param cmdCtx the command context to get the CommandSender from
* @return a Bukkit CommandSender from the provided Brigadier CommandContext
* @return a command sender from the provided Brigadier CommandContext
*/
public static <CommandSender> CommandSender getCommandSenderFromContext(CommandContext cmdCtx) {
CommandAPIPlatform<?, CommandSender, ?> platform = (CommandAPIPlatform<?, CommandSender, ?>) CommandAPIHandler.getInstance().getPlatform();
// For some reason putting this on one line doesn't work - very weird
AbstractCommandSender<CommandSender> abstractSender = platform.getSenderForCommand(cmdCtx, false);
return abstractSender.getSource();
public static <CommandSender, Source> CommandSender getCommandSenderFromContext(CommandContext cmdCtx) {
CommandAPIPlatform<?, CommandSender, Source> platform = (CommandAPIPlatform<?, CommandSender, Source>) CommandAPIHandler.getInstance().getPlatform();
return platform.getCommandSenderFromCommandSource((Source) cmdCtx.getSource());
}
}
6 changes: 2 additions & 4 deletions commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.mojang.brigadier.Message;
import com.mojang.brigadier.exceptions.CommandSyntaxException;
import com.mojang.brigadier.exceptions.SimpleCommandExceptionType;
import dev.jorel.commandapi.commandsenders.AbstractPlayer;
import dev.jorel.commandapi.exceptions.WrapperCommandSyntaxException;

import java.util.ArrayList;
Expand Down Expand Up @@ -213,14 +212,13 @@ public static void reloadDatapacks() {
}

/**
* Updates the requirements required for a given player to execute a command.
* Updates the player's view of the requirements for them to execute a command.
*
* @param player the player whose requirements should be updated
*/
public static <CommandSender, Player extends CommandSender> void updateRequirements(Player player) {
@SuppressWarnings("unchecked")
CommandAPIPlatform<?, CommandSender, ?> platform = (CommandAPIPlatform<?, CommandSender, ?>) CommandAPIHandler.getInstance().getPlatform();
platform.updateRequirements((AbstractPlayer<?>) platform.wrapCommandSender(player));
platform.updateRequirements(player);
}

// Produce WrapperCommandSyntaxException
Expand Down
164 changes: 42 additions & 122 deletions commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,14 @@

import java.util.ArrayList;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Optional;

import com.mojang.brigadier.LiteralMessage;
import com.mojang.brigadier.exceptions.CommandSyntaxException;
import com.mojang.brigadier.exceptions.SimpleCommandExceptionType;

import dev.jorel.commandapi.commandsenders.*;
import dev.jorel.commandapi.exceptions.WrapperCommandSyntaxException;
import dev.jorel.commandapi.executors.ExecutionInfo;
import dev.jorel.commandapi.executors.ExecutorType;
import dev.jorel.commandapi.executors.NormalExecutor;
import dev.jorel.commandapi.executors.ResultingExecutor;
import dev.jorel.commandapi.executors.TypedExecutor;

/**
Expand All @@ -42,143 +38,67 @@
* executors) and switches its execution implementation based on the provided
* command executor types.
*
* @param <CommandSender> The CommandSender for this executor
* @param <WrapperType> The AbstractCommandSender that wraps the CommandSender
* @param <CommandSender> The class for running platform commands
*/
public class CommandAPIExecutor<CommandSender, WrapperType
/// @cond DOX
extends AbstractCommandSender<? extends CommandSender>
/// @endcond
> {
public class CommandAPIExecutor<CommandSender> {

private List<NormalExecutor<CommandSender, WrapperType>> normalExecutors;
private List<ResultingExecutor<CommandSender, WrapperType>> resultingExecutors;
// Setup executors
private List<TypedExecutor<CommandSender, ? extends CommandSender, ?>> executors;

public CommandAPIExecutor() {
normalExecutors = new ArrayList<>();
resultingExecutors = new ArrayList<>();
this.executors = new ArrayList<>();
}

@SuppressWarnings("unchecked")
public void addNormalExecutor(NormalExecutor<?, ?> executor) {
this.normalExecutors.add((NormalExecutor<CommandSender, WrapperType>) executor);
public void addExecutor(TypedExecutor<CommandSender, ? extends CommandSender, ?> executor) {
this.executors.add(executor);
}

@SuppressWarnings("unchecked")
public void addResultingExecutor(ResultingExecutor<?, ?> executor) {
this.resultingExecutors.add((ResultingExecutor<CommandSender, WrapperType>) executor);
public void setExecutors(List<TypedExecutor<CommandSender, ? extends CommandSender, ?>> executors) {
this.executors = executors;
}

public int execute(ExecutionInfo<CommandSender, WrapperType> info) throws CommandSyntaxException {
// Parse executor type
if (!resultingExecutors.isEmpty()) {
// Run resulting executor
try {
return execute(resultingExecutors, info);
} catch (WrapperCommandSyntaxException e) {
throw e.getException();
} catch (Throwable ex) {
CommandAPI.getLogger().severe("Unhandled exception executing '" + info.args().fullInput() + "'", ex);
if (ex instanceof Exception) {
throw ex;
} else {
throw new RuntimeException(ex);
}
}
} else {
// Run normal executor
try {
return execute(normalExecutors, info);
} catch (WrapperCommandSyntaxException e) {
throw e.getException();
} catch (Throwable ex) {
CommandAPI.getLogger().severe("Unhandled exception executing '" + info.args().fullInput() + "'", ex);
if (ex instanceof Exception) {
throw ex;
} else {
throw new RuntimeException(ex);
}
}
}
}

private int execute(List<? extends TypedExecutor<CommandSender, WrapperType>> executors, ExecutionInfo<CommandSender, WrapperType> info)
throws WrapperCommandSyntaxException {
if (isForceNative()) {
return execute(executors, info, ExecutorType.NATIVE);
} else if (info.senderWrapper() instanceof AbstractPlayer && matches(executors, ExecutorType.PLAYER)) {
return execute(executors, info, ExecutorType.PLAYER);
} else if (info.senderWrapper() instanceof AbstractEntity && matches(executors, ExecutorType.ENTITY)) {
return execute(executors, info, ExecutorType.ENTITY);
} else if (info.senderWrapper() instanceof AbstractConsoleCommandSender && matches(executors, ExecutorType.CONSOLE)) {
return execute(executors, info, ExecutorType.CONSOLE);
} else if (info.senderWrapper() instanceof AbstractBlockCommandSender && matches(executors, ExecutorType.BLOCK)) {
return execute(executors, info, ExecutorType.BLOCK);
} else if (info.senderWrapper() instanceof AbstractProxiedCommandSender && matches(executors, ExecutorType.PROXY)) {
return execute(executors, info, ExecutorType.PROXY);
} else if (info.senderWrapper() instanceof AbstractRemoteConsoleCommandSender && matches(executors, ExecutorType.REMOTE)) {
return execute(executors, info, ExecutorType.REMOTE);
} else if (info.senderWrapper() instanceof AbstractFeedbackForwardingCommandSender && matches(executors, ExecutorType.FEEDBACK_FORWARDING)) {
return execute(executors, info, ExecutorType.FEEDBACK_FORWARDING);
} else if (matches(executors, ExecutorType.ALL)) {
return execute(executors, info, ExecutorType.ALL);
} else {
throw new WrapperCommandSyntaxException(new SimpleCommandExceptionType(
new LiteralMessage(CommandAPI.getConfiguration().getMissingImplementationMessage()
.replace("%s", info.sender().getClass().getSimpleName().toLowerCase())
.replace("%S", info.sender().getClass().getSimpleName()))).create());
}
}

private int execute(List<? extends TypedExecutor<CommandSender, WrapperType>> executors,
ExecutionInfo<CommandSender, WrapperType> info, ExecutorType type) throws WrapperCommandSyntaxException {
for (TypedExecutor<CommandSender, WrapperType> executor : executors) {
if (executor.getType() == type) {
return executor.executeWith(info);
}
}
throw new NoSuchElementException("Executor had no valid executors for type " + type.toString());
}

public List<NormalExecutor<CommandSender, WrapperType>> getNormalExecutors() {
return normalExecutors;
}

public List<ResultingExecutor<CommandSender, WrapperType>> getResultingExecutors() {
return resultingExecutors;
public List<TypedExecutor<CommandSender, ? extends CommandSender, ?>> getExecutors() {
return this.executors;
}

public boolean hasAnyExecutors() {
return !(normalExecutors.isEmpty() && resultingExecutors.isEmpty());
}

public boolean isForceNative() {
return matches(normalExecutors, ExecutorType.NATIVE) || matches(resultingExecutors, ExecutorType.NATIVE);
return !executors.isEmpty();
}

private boolean matches(List<? extends TypedExecutor<?, ?>> executors, ExecutorType type) {
for (TypedExecutor<?, ?> executor : executors) {
if (executor.getType() == type) {
return true;
// Use executors
public int execute(ExecutionInfo<CommandSender, ?> info) throws CommandSyntaxException {
try {
for (TypedExecutor<CommandSender, ? extends CommandSender, ?> executor : executors) {
Optional<Integer> result = tryExecutor(executor, info);

if (result.isPresent()) return result.get();
}
} catch (WrapperCommandSyntaxException e) {
throw e.getException();
} catch (Throwable ex) {
CommandAPI.getLogger().severe("Unhandled exception executing '" + info.args().fullInput() + "'", ex);
if (ex instanceof Exception) {
throw ex;
} else {
throw new RuntimeException(ex);
}
}
return false;
}

CommandAPIExecutor<CommandSender, WrapperType> mergeExecutor(CommandAPIExecutor<CommandSender, WrapperType> executor) {
CommandAPIExecutor<CommandSender, WrapperType> result = new CommandAPIExecutor<>();
result.normalExecutors = new ArrayList<>(normalExecutors);
result.resultingExecutors = new ArrayList<>(resultingExecutors);
result.normalExecutors.addAll(executor.normalExecutors);
result.resultingExecutors.addAll(executor.resultingExecutors);
return result;
// Executor not found
throw new SimpleCommandExceptionType(new LiteralMessage(
CommandAPI.getConfiguration().getMissingImplementationMessage()
.replace("%s", info.sender().getClass().getSimpleName().toLowerCase())
.replace("%S", info.sender().getClass().getSimpleName())
)).create();
}

public void setNormalExecutors(List<NormalExecutor<CommandSender, WrapperType>> normalExecutors) {
this.normalExecutors = normalExecutors;
}
// This needs to be extracted into another method to name the `Sender` and `Source` generic, which allows `executeWith` to accept the converted info
private <Sender extends CommandSender, Source> Optional<Integer> tryExecutor(TypedExecutor<CommandSender, Sender, Source> executor, ExecutionInfo<CommandSender, ?> info) throws WrapperCommandSyntaxException {
ExecutionInfo<Sender, Source> convertedInfo = executor.tryForSender((ExecutionInfo<CommandSender, Source>) info);

public void setResultingExecutors(List<ResultingExecutor<CommandSender, WrapperType>> resultingExecutors) {
this.resultingExecutors = resultingExecutors;
if (convertedInfo != null) {
return Optional.of(executor.executeWith(convertedInfo));
}
return Optional.empty();
}
}
70 changes: 17 additions & 53 deletions commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import com.mojang.brigadier.tree.ArgumentCommandNode;
import com.mojang.brigadier.tree.LiteralCommandNode;
import dev.jorel.commandapi.arguments.*;
import dev.jorel.commandapi.commandsenders.AbstractCommandSender;
import dev.jorel.commandapi.exceptions.CommandConflictException;
import dev.jorel.commandapi.executors.CommandArguments;
import dev.jorel.commandapi.executors.ExecutionInfo;
Expand Down Expand Up @@ -271,33 +270,18 @@ private void ensureNoCommandConflict(CommandNode<Source> nodeToRegister, Command
* @param executor Code to run when the command is executed.
* @return A Brigadier Command object that runs the given execution with the given arguments as input.
*/
public Command<Source> generateBrigadierCommand(List<Argument> args, CommandAPIExecutor<CommandSender, AbstractCommandSender<? extends CommandSender>> executor) {
public Command<Source> generateBrigadierCommand(List<Argument> args, CommandAPIExecutor<CommandSender> executor) {
// We need to make sure our arguments list is never changed
// If we just used the reference to the list, the caller might add arguments that aren't actually previous
// arguments for this suggestion node, and we would be confused because the new arguments don't exist
List<Argument> immutableArguments = List.copyOf(args);
// Generate our command from executor
return cmdCtx -> {
// Construct the execution info
AbstractCommandSender<? extends CommandSender> sender = platform.getSenderForCommand(cmdCtx, executor.isForceNative());
CommandSender sender = platform.getCommandSenderFromCommandSource(cmdCtx.getSource());
CommandArguments commandArguments = argsToCommandArgs(cmdCtx, immutableArguments);

ExecutionInfo<CommandSender, AbstractCommandSender<? extends CommandSender>> executionInfo = new ExecutionInfo<>() {
@Override
public CommandSender sender() {
return sender.getSource();
}

@Override
public AbstractCommandSender<? extends CommandSender> senderWrapper() {
return sender;
}

@Override
public CommandArguments args() {
return commandArguments;
}
};
ExecutionInfo<CommandSender, Source> executionInfo = new ExecutionInfo<>(sender, commandArguments, cmdCtx);

// Apply the executor
return executor.execute(executionInfo);
Expand Down Expand Up @@ -375,7 +359,7 @@ public SuggestionProvider<Source> generateBrigadierSuggestions(List<Argument> pr
return (context, builder) -> {
// Construct the suggestion info
SuggestionInfo<CommandSender> suggestionInfo = new SuggestionInfo<>(
platform.getCommandSenderFromCommandSource(context.getSource()).getSource(),
platform.getCommandSenderFromCommandSource(context.getSource()),
argsToCommandArgs(context, immutableArguments), builder.getInput(), builder.getRemaining()
);

Expand All @@ -393,44 +377,24 @@ public SuggestionProvider<Source> generateBrigadierSuggestions(List<Argument> pr
* @return A Predicate that makes sure a Brigadier source object satisfies the given permission and arbitrary requirements.
*/
public Predicate<Source> generateBrigadierRequirements(CommandPermission permission, Predicate<CommandSender> requirements) {
// If requirements are always false, result is always false
if (requirements == CommandPermission.FALSE()) return CommandPermission.FALSE();

// Find the intial check for the given CommandPermission
Predicate<AbstractCommandSender<? extends CommandSender>> senderCheck;
if (permission.equals(CommandPermission.NONE)) {
// No permissions always passes
senderCheck = null;
} else if (permission.equals(CommandPermission.OP)) {
// Check op status
senderCheck = AbstractCommandSender::isOp;
} else {
Optional<String> permissionStringWrapper = permission.getPermission();
if (permissionStringWrapper.isPresent()) {
String permissionString = permissionStringWrapper.get();
// check permission
senderCheck = sender -> sender.hasPermission(permissionString);
} else {
// No permission always passes
senderCheck = null;
}
}
Predicate<CommandSender> senderCheck = platform.getPermissionCheck(permission);

if (senderCheck == null) {
// Short circuit permissions check if it doesn't depend on source
if (permission.isNegated()) {
// A negated NONE permission never passes
return source -> false;
} else {
// Only need to check the requirements
return source -> requirements.test(platform.getCommandSenderFromCommandSource(source).getSource());
}
}
// Merge with requirements (unless its always true, which wouldn't add anything)
if (requirements != CommandPermission.TRUE()) senderCheck = senderCheck.and(requirements);

// Negate permission check if appropriate
Predicate<AbstractCommandSender<? extends CommandSender>> finalSenderCheck = permission.isNegated() ? senderCheck.negate() : senderCheck;
// Short circuit if the final test is always true or false
if (senderCheck == CommandPermission.TRUE()) return CommandPermission.TRUE();
if (senderCheck == CommandPermission.FALSE()) return CommandPermission.FALSE();

// Merge permission check and requirements
// Otherwise, convert Brigadier Source to CommandSender and run the check
final Predicate<CommandSender> finalSenderCheck = senderCheck;
return source -> {
AbstractCommandSender<? extends CommandSender> sender = platform.getCommandSenderFromCommandSource(source);
return finalSenderCheck.test(sender) && requirements.test(sender.getSource());
CommandSender sender = platform.getCommandSenderFromCommandSource(source);
return finalSenderCheck.test(sender);
};
}

Expand Down
Loading

0 comments on commit 3077c34

Please sign in to comment.