Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command Visibilty Conditions #348

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

PoQuatre
Copy link

Hi! I added this feature for one of my plugins, so I thought why not make a PR for it because people might find useful.

As the title suggest it adds a way to programmatically show or hide a subcommand, it only change the tab completion like @Private.

How it works

  1. You register a condition :
commandManager.getCommandVisibilityConditions().addCondition("creative", (context) -> {
    if (!context.getIssuer().isPlayer()) return true;
    return context.getIssuer().getPlayer().getGameMode() == GameMode.CREATIVE;
});

commandManager.getCommandVisibilityConditions().addCondition("flying", (context) -> {
    if (!context.getIssuer().isPlayer()) return true;
    return context.getIssuer().getPlayer().isFlying();
});
  1. You use it with either @ShowCommand or @HideCommand :
@Subcommand("example1")
@ShowCommand("creative")
public void firstExample(CommandSender sender) {
    ...
}

@Subcommand("example2")
@HideCommand("creative")
public void secondExample(CommandSender sender) {
    ...
}

@Subcommand("example3")
@ShowCommand("creative")
@HideCommand("flying")
public void thirdExample(CommandSender sender) {
    ...
}
  1. That's all you have to do to make it work.

So what is it doing in game :

  • In the first example, the subcommand example1 is only shown in the tab completion if the player is in gamemode creative, this is because with @ShowCommand the subcommand is hidden by default and is only shown when all conditions return true.
  • In the second example, the subcommand example2 is only shown in the tab completion if the player is not in gamemode creative, this is because with @HideCommand the subcommand is shown by default and is only hidden when all conditions return true.
  • And in the third example, the subcommand example3 is only shown in the tab completion if the player is in gamemode creative and not flying, this is because @HideCommand takes precedence over @ShowCommand.

Condition Configs

Like normal conditions, configs are available here is an example :

commandManager.getCommandVisibilityConditions().addCondition("height", (context) -> {
    if (!context.getIssuer().isPlayer()) return true;

    final int min = context.getConfigValue("min", -1);
    final int max = context.getConfigValue("max", -1);
    final double height = context.getIssuer().getPlayer().getHeight();

    return min < height && (max == -1 || max > height);
});
@Subcommand("example")
@ShowCommand("height:min=64,max=128")
public void example(CommandSender sender) {
    ...
}

In this example the subcommand example is only shown when the player is between Y 64 and Y 128.

Note : You can chain conditions using | between them.

Copy link
Collaborator

@chickeneer chickeneer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of code analysis issues. But big picture. I don't see any real advantages to the Hide condition, based on your description and code analysis it appears redundant in terms of functionality.
Since all Show conditions are required and only one hide condition can prevent seeing. They are effectively the same just different names. Only advantage being the API level of how creating the commands feels which has value. But the flip to that - is that it introduces questions about how stuff interacts together.

Thinking about it further even. I would kinda wish it was integrated more directly with the Private annotation. As it is is for the same type of behavior. "Private unless this condition is met to show".

Open to anyone elses thoughts on the matter - I don't see us merging this as is. I definitely see the advantage of having conditions for visibility though.


private boolean validateShowConditions(RegisteredCommand cmd, CommandIssuer issuer) {
return validateConditions(cmd, cmd.showConditions, issuer, true)
&& validateShowConditions(cmd, cmd.scope, issuer);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming I am not crazy, validateShowConditions used here also validateConditions in the same way as line 45 on Line 59.


private boolean validateHideConditions(RegisteredCommand cmd, CommandIssuer issuer) {
return validateConditions(cmd, cmd.hideConditions, issuer, false)
|| validateHideConditions(cmd, cmd.scope, issuer);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here, validateHideConditions also validateConditions twice, lines 64/69.

CC conditionContext = (CC) this.manager.createConditionContext(issuer, config);
results[i] = visibilityCondition.validateCondition(conditionContext);
}
return Arrays.stream(results).allMatch(Boolean.TRUE::equals);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there rationale for validating all of the conditions when they all have to be true?

/**
* Specifies visibility conditions that must be met in order to hide this
* subcommand from the tab completion. If the conditions are met it will
* overwrite {@code @ShowCommand}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be ShowConditions link.

@DevJoey
Copy link

DevJoey commented Jan 26, 2023

Nice feature would be very useful to have more control over that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants