TriggerReactor

TriggerReactor

24.6k Downloads

Registration and Tab Completion support for Command Trigger

MidnightSugar opened this issue ยท 26 comments

commented

A few problems I have found while using command triggers are as follows:

  • Third Party Plugins are unable to use custom commands from command triggers.
  • Custom commands do not have any kind of tab complete and show as red in game.

I recently found a plugin called MyCommands found here: https://www.spigotmc.org/resources/mycommand.22272/

Looking through the configs I discovered that has function to register a command and customize its tab complete entirely through its configuration files. I believe if TR added this to its command triggers then it would be awesome. Each command trigger comes with its own separate yml file, I am thinking these kind of settings for registering and tab complete can be put here. Here is a snippet from its wiki to explain how it works:

Register Real commands and the Tab Completer

Why register a command?
Registering a command, give you some vantages, like the ability to perform this new created commands in another plugin.
Another plus is, the Tab Completer. This is available only if the command is registered.

So, how to register a command then?

You can do that, by adding the command field : "register: true" or "registered: true" inside the command you want to register.

Example :

'registered_command':
command: /buycraft-pack1
type: RUN_COMMAND
runcmd:
- "/say The player $arg1, have just buyed the Pack1 from the store!"
register: true

Another one, this time with the TabCompleter

'gm_alias':
command: /gm
type: ALIAS
alias: /gamemode
register: true
tab_completer:
- survival
- creative
- adventure

In this example, you can see the TAB Completer. This function works only if the command is registered. Like the example, if you type in the game chat "/gamemode" and then press TAB, it will return to you with the tips inside the tab_completer command field, so "survival, creative ecc".

By default, if you don't set the tab_completer field in the command, when you press TAB it will return with the list of the connected players. For now, you can't have both.

Advanced use of the TabCompleter (>5.5.1) :

tab_completer can be customized by positions following the scheme here forward.

There is a special PlaceHolder for the tab_completer only : $player_list (It returns with a list of the online players)

tab_completer:
1:
- set
- add
- remove
2:
- $player_list
3:
- money
- gold
- credits
4:
- amount
- $rnd6

In this example instead, every position of the command typed in the chat prompt, will return with different informations. ALWAYS use numbers in an ascending order 1,2,3... .

Example, if in the chat i have "/manageuser " where "" is the prompt position, the auto completer above the chat will show me "set , add and remove". Again, if i have "/manageuser set _", in the following case, i will have the list of the connected players in the server, because the placeholder "$player_list" does this. And so on.

P.S.You can't register (REGISTER : TRUE ONLY) commands with spaces in the name.
command: /test example (WRONG) -> command: /testexample (CORRECT)

REGISTERED commands can't be created with multiple arguments in the command field, only non-registered ones can.`

commented

@MidnightSugar
I agree with the suggestion about registering commands. Valid commands being highlighted red is annoying. However, the real reason why 3rd party plugins won't execute trg commands is that trg commands only respond if a player runs it.
The current implementation of command triggers intercepts any commands run by players and checks to see if it's a trg command. If it is, it cancels the command and runs your script instead.

https://www.spigotmc.org/threads/small-easy-register-command-without-plugin-yml.38036/
Here's a thread about how to register commands without modifying plugin.yml in bukkit. It involves reflection
As for sponge, it seems that there's nothing stopping us from registering new commands after startup with the CommandManager, which is the class we already use for /trg.

I disagree with many aspects of the tab_completer suggestion though:

There is a special PlaceHolder for the tab_completer only : $player_list (It returns with a list of the online players)
Now we have 2 parallel sets of placeholders: the ones you use in scripts, and the ones you use in the tab completer. It's going to be very complicated to introduce a second set of placeholders. Also, it breaks the entire logic of the configuration structure. Each completion is supposed to be a single item, but now you've created a placeholder which represents an entire list of items.
Tab completion placeholders probably aren't realistic for trg.

Also, this scheme doesn't work for subcommands.
for a simple example, think about bedrock edition's time command:

time add <amount>
time set <amount>
time query <daytime|gametime|day>

Better yet, think about how you would write tab completion for /trg.

I believe that the only way to support every kind of tab completion for every type of command would be to add a new kind of trigger that does tab completion.
Tab completion isn't a major feature though, so it isn't worth adding a whole new trigger to support it. Writing triggers also isn't as simple as changing the config.
So we need to come up with a format that:

  • works for most types of commands, including subcommands
  • is simple.
  • doesn't use too many magic strings
commented

But internally supporting such feature would be handy for the end-users

Just my thought

commented

Or what if we changed it to use nested yaml lists: https://interviewbubble.com/nested-lists-in-yaml-yaml-examples/

So the time command would be a simple:

Tabcomplete:
- add
- set
- query
- - daytime
  - gametime
  - day
commented

Could custom triggers be used in any way for setting tab complete?

commented

You can already use them to.
Just listen to the tab complete event.

commented

What if we also allowed this format:

- set, add, remove
- - $playerlist
- - - money, gold, credits
commented

That might work, as long as we hope that no one ever tries to use a completion that starts with a - or contains a comma.
It might still cause duplication if the command syntax splits then converges again:

/trg custom <custom trigger> delete/edit/create
/trg click <click trigger> delete/edit/create
- custom
- -<custom trigger names>
- - - delete
- - - edit
- - - create
- click
- -<click trigger names>
- - - delete
- - - edit
- - - create

Hopefully this doesn't happen much in practice, but I have an idea for what the format should be to avoid these kinds of problems. I took some inspiration from your original idea:

Tabcomplete:
  1:
    - delete
    - edit
    - create
  0:
    - custom
    - - <custom trigger names>
    - - - $1
    - click
    - - <click trigger names>
    - - - $1

In this case, $1 represents the list starting at 1:
The cool thing about this is that it prevents almost every kind of duplication.
A defined list of completions can be used anywhere

You could also make variable-length tab completion with recursion:
This example makes a tab completion that accepts a list of players:

0:
  - $player_list
  - - $0

Either way, we need to decide on something else:
how do you indicate an argument that should have no completions?

commented

I'm going to assign this to 3.1, but only the first suggestion about registering commands

commented

but then your original command would become:

- set
- -$player_list
- - - money
- - - gold
- - - credits
- add
- -$player_list
- - - money
- - - gold
- - - credits
- remove
- -$player_list
- - - money
- - - gold
- - - credits
commented

I have another idea. What if we just literally format it like a normal help command:

Tabcomplete:
- delete/edit custom $customtriggers
- delete/edit click $clicktriggers
- create custom/click
commented

I don't understand this format, can you explain more?

commented

The space will seperate the different slot position. So first argument will be slot 1, second argument slot 2, etc. The slash will seperate different tabcompletes for a single argument. Each separate list would be a new subcommand with different tabcomplete.

commented

So far, My favorite has been:

- set, add, remove
- - $playerlist
- - - money, gold, credits

It should be good enough for most commands, and not too hard to implement
What's your call @wysohn

commented

@gerzytet
Looks like a solid solution for me

it would be straight forward to implement since we already have yml file for the command triggers.

commented

I still think my last suggestion is the best. It would work exactly the same as that one, but much easier to type out. Instead of using nested yaml, it would simply use spaces. Also much easier to read.

commented
Tabcomplete:
- set/add/remove $playerlist money/gold/credits

All on one line and no use of crazy - structures

Also we could make it so any text surrounded by <> would tell the plugin it is an argument with no tabcomplete.

Like:

- give $material <amount> $playerlist
commented

somecommand.json

{
	"sync": false,
	"aliases": ["this", "andthat"],
	"permissions": ["x.y.z", "y.z.z"],
	"tabs": [{
			"hint": "<set/add/remove>",
			"candidates": "set,add,remove"
		},
		{
			"hint": "<player name>",
			"candidates": "$playerlist"
		},
		{
			"hint": "<money/gold/credit>",
			"candidates": "money,gold,credit"
		},
		{
			"hint": "amount"
		}
	]
}

Starting from 3.1.0, we are moving from yaml to json (using Gson).

commented

Design Proposal


tab_completer

commented

It looks good to me. This may be going a bit out of scope of this issue, but if we are discussing design may I suggest a couple things to the json which I would personally mind very useful. Some of these probably have been mentioned before:

  1. Add "enable: true"

This would allow me to easily disable scripts without having to delete them. Technically I could add #STOP at the beginning, but this would be easier when used in conjunction with my other ideas.

  1. Require names for all new triggers/scripts. It is extremely difficult to navigate scripts if they do not have a proper name. Once we start requiring names, then we can start designing stuff around them. I have to open like 10 different files to find the one I want to use. Sure, there is the in game tools, but those should not be the only way of finding scripts.

You would probably wouldn't want to force this change, because it could be tedious to add names if you have a ton of scripts, so maybe add in legacy support for have no names, so scripts will still work for existing scripts. This would only be required for newly created ones. Maybe a tool could be made where you click on a walk or click trigger and it will allow you to edit settings for it, one of them being the name, or being able to easily change name in file.

Probably would make sense if every script (even walk triggers) should come with a gson file. Then location information could be put inside the gson instead. The script name would be the file name for every script.

  1. Add in more commands, or possibly a menu for editing the yaml or now gson file. Like /trg cmd <cmd> setpermission. /trg cmd <cmd> setalias. /trg cmd <cmd> settab and /trg cmd <cmd> disable. This would complement the in-game editor.
commented

@MidnightSugar
Will have to put that in separate issues

Do you want me to open it up, or do you want to do it yourself?

commented

@wysohn Feel free to organize it as you wish. I don't mind.

commented

It seems like the new commands are registered only on the server start

commented

Though for suggestion 1. there is no clean way to support it (we could, if we work on NMS, but I do not want it), so let's add a command called /trg runcmd <command trigger> so that other plugins can use it.

commented

@gerzytet @MidnightSugar
Can we actually change the structure a little bit?

- - set
  - add
  - remove
- $playerlist
- - money
  - gold
  - credits

We can make it a list of list of strings or string

Screenshot from 2020-03-20 23-48-50

commented

According to the website I linked, both of those formats work the same.

See example 4 on https://interviewbubble.com/nested-lists-in-yaml-yaml-examples/

commented

@MidnightSugar
Yeah, yet @gerzytet once said he likes the stair-like structure the best, and I find it not so straight forward when it comes to implementing it. I'm waiting for his opinion too