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

Add variable length parameter #275

Closed
SNWCreations opened this issue May 1, 2022 · 12 comments
Closed

Add variable length parameter #275

SNWCreations opened this issue May 1, 2022 · 12 comments
Labels
enhancement New feature or request

Comments

@SNWCreations
Copy link

I would like to add a variable-length parameter that wraps all known parameters (except itself).
It is like GreedyStringArgument, but it is enchanced.

Please supply examples of the desired inputs and outputs
Example:
Input: /test a
Output: a
But if we use /test a b c, it will say "a,b,c"
Or "/test 0 1 2", it will say "0,1,2"

Example for Developers:

new CommandAPICommand("test")
  .withArguments(
    new VariableLengthParameter("names", Object.class)
	// or the class of special arguments, this allows you to check whether the user-supplied parameters are instances of these classes.
	// I use Object.class to allow all possible argument.
  ).executes((sender, args) -> {
    Collection<Object> result = (Collection<Object>) args[0]; // or Object[]
	String message = String.join(",", Arrays.stream(result).map(IT -> String.valueOf(IT)).collect(Collectors.toList()).toArray());
	sender.sendMessage(message);
  }).register();
@SNWCreations SNWCreations added the enhancement New feature or request label May 1, 2022
@JorelAli
Copy link
Owner

JorelAli commented May 2, 2022

This request confuses me. What are you trying to achieve that a GreedyStringArgument doesn't already achieve on its own?

Is this not the best option for your situation?

new CommandAPICommand("test")
	.withArguments(new GreedyStringArgument("names"))
	.executes((sender, args) -> {
		String names = (String) args[0];
		String commaSeparatedNames = Strings.join(Arrays.asList(names.split(" ")), ',');
	})
	.register();

@SNWCreations
Copy link
Author

SNWCreations commented May 2, 2022

No.
I will tell you why I want this feature.
I'm working on a minigame plugin (see https://github.com/SNWCreations/RunForMoney) , and I defined a command called "hunter" in that plugin.
That command can put players into a team (this team system is not Minecraft "team").
And this command allow administrators put the players into the team by force. Just need provide a list of player names through the arguments.
It all sounds like a case for GreedyStringArgument, but the problem is in the advice section.
I hope to provide different suggestions after the player types the space.
such as "/hunter PlayerA"
When a player puts a space after PlayerA, a list of player names should be provided.
So if I want to implement that feature, I will need this:
(Just a copy)

new CommandAPICommand("hunter")
  .withArguments(
    new VariableLengthParameter("names", Player.class).replaceSuggestions(...) // how can I write it?
  ).executes((sender, args) -> {
    Collection<Player> result = (Collection<Player>) args[0];
    for (Player a : result) {
        TeamHolder.getInstance().addHunter(a);
        // some messages will sent to the CommandSender
    }
  }).register();

@JorelAli
Copy link
Owner

JorelAli commented May 2, 2022

@SNWCreations This isn't something that requires a new argument - all you would need is a suitable suggestion function for a GreedyStringArgument. For example, this is what you may have to write:

new CommandAPICommand("hunter")
	.withArguments(new GreedyStringArgument("names").replaceSuggestions(ArgumentSuggestions.strings(info -> {
		String currentArg = info.currentArg();
		
		// If we end with a space, we prompt for the next player name
		if(currentArg.endsWith(" ")) {
			Set<String> players = new HashSet<>();
			for(Player player : Bukkit.getOnlinePlayers()) {
				players.add(player.getName());
			}
			
			players.removeAll(Arrays.asList(currentArg.split(" ")));
			
			// 'players' now contains a set of all players that are NOT in
			// the current list that the user is typing. We want to return
			// a list of the current argument + each player that isn't
			// in the list (i.e. each player in 'players')
			return players.stream().map(player -> currentArg + player).toArray(String[]::new);
		} else {
			
			// Auto-complete the current player that the user is typing
			// Remove the last argument and turn it into a string as the base for suggestions
			List<String> currentArgList = new ArrayList<>(Arrays.asList(currentArg.split(" ")));
			currentArgList.remove(currentArgList.size() - 1);
			String suggestionBase = currentArgList.isEmpty() ? "" : currentArgList.stream().collect(Collectors.joining(" ")) + " ";


			return Bukkit.getOnlinePlayers().stream()
					.filter(player -> player.getName().startsWith(currentArg.split(" ")[currentArg.split(" ").length - 1]))
					.map(playerName -> suggestionBase + playerName)
					.toArray(String[]::new);
		}
	})))
	.executes((sender, args) -> {
		String[] names = ((String) args[0]).split(" ");
		Player[] players = Bukkit.getOnlinePlayers().stream()
			.filter(player -> Arrays.asList(names).contains(player))
			.toArray(Player[]::new);
	})
	.register();

(Note: I've not tested this!)

@SNWCreations
Copy link
Author

Thanks! I will test it soon.

@SNWCreations
Copy link
Author

@SNWCreations This isn't something that requires a new argument - all you would need is a suitable suggestion function for a GreedyStringArgument. For example, this is what you may have to write:

new CommandAPICommand("hunter")
	.withArguments(new GreedyStringArgument("names").replaceSuggestions(ArgumentSuggestions.strings(info -> {
		String currentArg = info.currentArg();
		
		// If we end with a space, we prompt for the next player name
		if(currentArg.endsWith(" ")) {
			Set<String> players = new HashSet<>();
			for(Player player : Bukkit.getOnlinePlayers()) {
				players.add(player.getName());
			}
			
			players.removeAll(Arrays.asList(currentArg.split(" ")));
			
			// 'players' now contains a set of all players that are NOT in
			// the current list that the user is typing. We want to return
			// a list of the current argument + each player that isn't
			// in the list (i.e. each player in 'players')
			return players.stream().map(player -> currentArg + player).toArray(String[]::new);
		} else {
			
			// Auto-complete the current player that the user is typing
			// Remove the last argument and turn it into a string as the base for suggestions
			List<String> currentArgList = new ArrayList<>(Arrays.asList(currentArg.split(" ")));
			currentArgList.remove(currentArgList.size() - 1);
			String suggestionBase = currentArgList.isEmpty() ? "" : currentArgList.stream().collect(Collectors.joining(" ")) + " ";


			return Bukkit.getOnlinePlayers().stream()
					.filter(player -> player.getName().startsWith(currentArg.split(" ")[currentArg.split(" ").length - 1]))
					.map(playerName -> suggestionBase + playerName)
					.toArray(String[]::new);
		}
	})))
	.executes((sender, args) -> {
		String[] names = ((String) args[0]).split(" ");
		Player[] players = Bukkit.getOnlinePlayers().stream()
			.filter(player -> Arrays.asList(names).contains(player))
			.toArray(Player[]::new);
	})
	.register();

(Note: I've not tested this!)

@JorelAli
Emmm. I'm sorry, but I have to reopen this.
Sorry, your code does not work correctly.
It works while my command have no arguments, like "/hunter "
When I press tab, my username has added.
I invited my friend to my server.
After that, I did this but the code does not work correcly.
I inputed "/hunter SNWCreations "
But when I press the tab, it is not "/hunter SNWCreations somebody"
It is "/hunter SNWCreations SNWCreations\ somebody".
I don't know why, and I pressed tab again after that
Now my command is "/hunter SNWCreations SNWCreations\ SNWCreations SNWCreations\ somebody"
What should I do?

@willkroboth
Copy link
Collaborator

This revised version of the code seems to be working for me:

new CommandAPICommand("hunter")
        .withArguments(new GreedyStringArgument("names").replaceSuggestions(ArgumentSuggestions.strings(info -> {
            String currentArg = info.currentArg();

            // If we end with a space, we prompt for the next player name
            if(currentArg.endsWith(" ")) {
                Set<String> players = new HashSet<>();
                for(Player player : Bukkit.getOnlinePlayers()) {
                    players.add(player.getName());
                }

                Arrays.asList(currentArg.split(" ")).forEach(players::remove);

                // 'players' now contains a set of all players that are NOT in
                // the current list that the user is typing. We want to return
                // a list of the current argument + each player that isn't
                // in the list (i.e. each player in 'players')
                return players.stream().map(player -> currentArg + player).toArray(String[]::new);
            } else {

                // Auto-complete the current player that the user is typing
                // Remove the last argument and turn it into a string as the base for suggestions
                List<String> currentArgList = new ArrayList<>(Arrays.asList(currentArg.split(" ")));
                String nameStart = currentArgList.remove(currentArgList.size() - 1);
                String suggestionBase = currentArgList.isEmpty() ? "" : String.join(" ", currentArgList) + " ";

                return Bukkit.getOnlinePlayers().stream()
                        .filter(player -> player.getName().startsWith(nameStart))
                        .map(player -> suggestionBase + player.getName())
                        .toArray(String[]::new);
            }
        })))
        .executes((sender, args) -> {
            String[] names = ((String) args[0]).split(" ");
            Player[] players = Bukkit.getOnlinePlayers().stream()
                    .filter(player -> Arrays.asList(names).contains(player.getName()))
                    .toArray(Player[]::new);
        })
        .register();

Revising the original suggestion, I think it would be useful if the CommandAPI had a way to do this more compactly. Maybe something like this:

new CommandAPICommand("hunter")
        .withArguments(new GreedyArgument<Player>("players", Bukkit::getOnlinePlayers, Player::getName))
        .executes((sender, args) -> {
            Collection<Player> result = (Collection<Player>) args[0];
        })
        .register();

This new GreedyArgument might have a constructor that looks something like this:

GreedyArgument(String nodeName, Supplier<Collection<T>> choices, Function<T, String> forCommand)

It takes in a function that supplies a collection of some type to choose from (in this case Bukkit::getOnlinePlayers), and a function that turns that type into a string to display in the options list (in this case Player::getName). Behind the scenes, the CommandAPI automatically does the suggestion replacing and adds a collection to the args array to pass into the execute function.

If the GreedyArgument doesn't fit into the Argument system very well, maybe adding a method to GreedyStringArgument or ArgumentSuggestions could do something similar.

@SNWCreations
Copy link
Author

I think your suggestion is also good.

Why I said the code is not working?
Because I am testing it in console. (That sounds like I'm stupid. lol)

@JorelAli
Copy link
Owner

JorelAli commented May 5, 2022

It takes in a function that supplies a collection of some type to choose from (in this case Bukkit::getOnlinePlayers), and a function that turns that type into a string to display in the options list (in this case Player::getName). Behind the scenes, the CommandAPI automatically does the suggestion replacing and adds a collection to the args array to pass into the execute function.

@willkroboth I quite like this idea. What we're effectively trying to achieve is some form of "list argument". The CommandAPI's SafeOverrideableArgument abstract class supports providing a mapping of some type T to a String which is used for suggestion purposes.

I can envision something along the lines of:

new ListArgument<Player>("players", " ", Player::getName)
    .replaceSafeSuggestions(SafeSuggestions.suggest(Bukkit::getOnlinePlayers));

Where the second parameter (in this example, " ") is the list delimiter. This would allow you to use any delimiter you want (for example, a comma ",").

@SNWCreations
Copy link
Author

SNWCreations commented May 6, 2022

It takes in a function that supplies a collection of some type to choose from (in this case Bukkit::getOnlinePlayers), and a function that turns that type into a string to display in the options list (in this case Player::getName). Behind the scenes, the CommandAPI automatically does the suggestion replacing and adds a collection to the args array to pass into the execute function.

@willkroboth I quite like this idea. What we're effectively trying to achieve is some form of "list argument". The CommandAPI's SafeOverrideableArgument abstract class supports providing a mapping of some type T to a String which is used for suggestion purposes.

I can envision something along the lines of:

new ListArgument<Player>("players", " ", Player::getName)
    .replaceSafeSuggestions(SafeSuggestions.suggest(Bukkit::getOnlinePlayers));

Where the second parameter (in this example, " ") is the list delimiter. This would allow you to use any delimiter you want (for example, a comma ",").

Wow! I like this! I think this can replace my original suggestion.
If this is possible, will it be available in v7? (I still working on 1.16.5!)

@JorelAli
Copy link
Owner

JorelAli commented May 6, 2022

If this is possible, will it be available in v7? (I still working on 1.16.5!)

@SNWCreations Why are you still using Minecraft 1.16.5? The CommandAPI is currently written in Java 17 and Minecraft 1.16.5 does not support Java 17. Backporting this feature is not only a vulnerability risk due to better security features in Java 17 and more powerful and performant features that are not present in Java 16.

Additionally, there are certain features in Minecraft 1.16.5 that cannot be backported easily and would require a great deal of development time on my behalf that I would rather dedicate to more interesting upcoming features.

Also, this feature is effectively syntactic sugar - it's possible to do this by writing your own methods.

@SNWCreations
Copy link
Author

SNWCreations commented May 6, 2022

@JorelAli

The reason is easy to understand. I need compatibility.
If it is impossible to provide in v7, how can I re-implement that?
I have a method to let Paper 1.16.5 run on Java 17. see https://gist.github.com/SNWCreations/5c4bdf152a5a992221684c0af853fd80.

@SNWCreations
Copy link
Author

SNWCreations commented May 6, 2022

The revised version of @JorelAli 's code by @willkroboth works. But it can't work correctly in console.
However, thanks!
My question is solved.
I think we can start a new Issue on the Issue of the list parameter, and this Issue can be closed.

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

No branches or pull requests

3 participants