eccentricdevotion / TARDIS

A Spigot / Paper plugin for all Doctor Who fans - create and use a TARDIS! It's bigger on the inside!
GNU General Public License v3.0
80 stars 28 forks source link

Enhancements to Handles! #217

Closed PrincessRTFM closed 4 years ago

PrincessRTFM commented 4 years ago

Enhancing Handles.... enhandles?


Describe the feature request

Three pieces this time. First: instead of splitting on spaces and checking for contents, what about regexen? hey handles could become (?:hey\s+handles(?:[,!:\s]|\.\.\.+)|handles(?:[,!:]|\.\.\.+)) for the prefix check, allowing all of the following to work for a more speech-like flow:

And then the commands could be similarly smoothed to make it more english-y. travel could become (?:travel|go\s+to) for:

and more could be added.


Second! More configurability, including aliases (admin-created), and moving all of the Handles stuff to handles.yml. For aliases, I mean something like this:

- search:
  - "regex1"
  - "regex2"
  - etc
  commands:
  - "command to run as the Timelord"
  - "another command to run as the Timelord"
  - etc

Optionally, command entries could include a permissions section (as a map of the permission node to the required value - true or false) to limit which Timelords can use it. There could also be entries in the file for the existing commands, to allow changing the triggers for them as well - at least, insofar as that can be done for ones which need to extract text. Maybe a capture group?


Third! And last! Hooking into PlaceholderAPI. Both for the command strings - so that, for instance, a command can be constructed like (this is just the first example I thought of) tell %player% I'm sorry %displayname%, I'm afraid I can't do that (HANDLES 9000 anyone?) and also providing placeholders. For instance, %timelord_artron_amount% and %timelord_artron_percent%. Well, I suppose providing placeholders like that isn't really a Handles thing, but it'd be absolutely awesome!


I know this is another big one, but I'm of the opinion that it's always better to offer the suggestions and details so the coder can decide for themselves if it's cool enough to be worth it.

eccentricdevotion commented 4 years ago

Great idea, but what happens to your regex when handles.prefix in the config is changed to something else? "oye manijas", "salut poignées", "excuse me handles" for instance. Or are you thinking that server admins would write their own expressions in handles.yml?

PrincessRTFM commented 4 years ago

I was thinking the prefix would be part of the handles.yml config, so that the structure might look (for example) something like...

prefix: "regex here"
core-commands:
  travel:
    random: "regex where capture group 1 is the destination"
    player: "regex where capture group 1 is the player's name"
    # etc, for the other travel commands
  door:
    open: "regex"
    close: "regex"
    lock: "regex"
    unlock: "regex"
  # any other core/built-in commands for Handles have their triggers defined as a regex here
custom-commands:
  - trigger: "search pattern to apply to everything AFTER the prefix"
    commands:
    - command 1
    - command 2
    - etc

Obviously, the exact structuring is up to you, but I was thinking that all of the configs for Handles would go in that file, like with the artron values. There could be an "enabled" option in there, or in the main file, or even just turn Handles off if the prefix is empty or false. For handling commands, if the prefix pattern matches, it would be replaced with an empty string, the remainder would be trimmed to remove whitespace on the ends, and then that would be matched against the core commands first and if none of them matched then against the custom ones second. If nothing matched, it'd return the "does not compute" message.

PrincessRTFM commented 4 years ago

Er, quick (immediate) followup: any of the regex scalars could, if you wanted to code for it, accept a scalar for one regex or a list for multiple, or could require a list and to give a single one you'd just.... give a single one, so that instead of really complicated expressions, there could be a set of simpler ones tried one after the other until a match or until it's out. Upside: less complicated regexen for more options, easier on the server admins using it. Downside: you'd have to code for multiple regexen and test serially until there's a match or the list runs out and you need to move on to the next one.

eccentricdevotion commented 4 years ago

So travel.player regex would be something like:

(?:travel\s+|go\sto\s+player\s+)([a-zA-Z_\d]+)

or:

- (?:travel\s+player\s+)([a-zA-Z_\d]+)
- (?:go\s+to\s+player\s+)([a-zA-Z_\d]+)
eccentricdevotion commented 4 years ago

My biggest worry is server admins f**king up the expressions... it could be incredibly powerful in the right hands, but how many 11 year old server ops know how to write a decent regex?

I guess if the defaults replicate the current setup then most people won't even bother looking at handles.yml at all...

eccentricdevotion commented 4 years ago

Happy to accept a PR if you want to have bash at it yourself :)

eccentricdevotion commented 4 years ago

bf96133 804b84f

PrincessRTFM commented 4 years ago

I was thinking the defaults would replicate the original setup, yeah. I might have a go, but it's been a long time since I've done java, I don't think I even have a minecraft dev setup anymore... I'd need to put one together first so I can check my work before submitting anything.

eccentricdevotion commented 4 years ago

OK handles.yml is looking like this now:

# must use single quotes to wrap regular expressions
enabled: true
prefix: '^(?:hey\s+)?handles(?:[,!:\s]|\.\.\.+)'
reminders:
  enabled: true
  schedule: 1200
core-commands:
  craft: '\b(?:craft|build|make)\b.*\b(\w+)\s+tardis\b'
  remind: 'remind\s?(?:me\s+to)?\s+(.+)\s+.+(\d+)'
  say: 'say\s+(.+)'
  name: 'name'
  time: 'time'
  call: 'call'
  takeoff: 'takeoff|take off'
  land: 'land'
  hide: 'hide'
  rebuild: 'rebuild'
  direction: '(?:fac(?:ing|e)|direction)\s+(\w+)\s*?'
  travel:
    save: '(?:travel|go).+save\s+(\w+)'
    home: '(?:travel|go)\s+home'
    random: '(?:travel|go|find).+random'
    player: '(?:travel|go).+player\s+(\w+)'
    area: '(?:travel|go).*(?:\b(\w+)\s+area)\s*?$|(?:travel|go).+(?:area\s\b(\w+))\s*?'
    biome: '(?:travel|go).*(?:\b(\w+)\s+biome)\s*?$|(?:travel|go).+(?:biome\s\b(\w+))\s*?'
    cave: '(?:travel|go|find).+cave'
    village: '(?:travel|go|find).+village'
  door:
    open: '\bopen\b.*\bdoor\b|\bdoor\b.*\bopen\b'
    close: '\bclose\b.*\bdoor\b|\bdoor\b.*\bclose\b'
    lock: '\block\b.*\bdoor\b|\bdoor\b.*\block\b'
    unlock: '\bunlock\b.*\bdoor\b|\bdoor\b.*\bunlock\b'
  scan: 'scan'
  teleport: 'teleport'
  transmat: 'transmat\s?(?:me\s+to)?\s+(.+)'
custom-commands:
  trigger:
    regex: 'search pattern to apply to everything AFTER the prefix'
    permisssion: some.permission
    commands:
      - command 1
      - command 2
      - etc

Probably some dodgy regex in there, it's definitely not my strong point, but have learnt a few new tricks :)

eccentricdevotion commented 4 years ago

ae1316c

PrincessRTFM commented 4 years ago

Oooh! Also, I'd change the transmat regex to transmat\s+(?:(?:me\s+)?to\s+(?:the\s+)?)?(.+) personally - remove a possible whitespace weirdness but also allow transmat <where>, transmat me to <where>, transmat to <where>, transmat me to the <where>, and transmat to the <where>. I usually say transmat to console personally.

Obviously it's customisable now, but that's just a suggestion for the default! This looks awesome, I'm really happy with it, thank you!

eccentricdevotion commented 4 years ago

68a662500

eccentricdevotion commented 4 years ago

Have you had a chance to test the custom-commands?

PrincessRTFM commented 4 years ago

Not yet, I've been unusually busy the last few days, but hopefully soon I can finally get back on my server to play with it!

eccentricdevotion commented 4 years ago

Not working yet anyway, fixing it now...

eccentricdevotion commented 4 years ago

48f9d69 Question though, I've added placeholder support to the [custom-]commands that handles will run, but I'm not sure it is what you intended...

tell %player% I'm sorry %displayname%, I'm afraid I can't do that

didn't work, it had to be: tell %player_name% I'm sorry %player_displayname%, I'm afraid I can't do that and then you get this:

tell

Is that what you want / expect?

PrincessRTFM commented 4 years ago

Ah, I had the wrong placeholder names, my bad... and yeah, that looks right. /tell is the private messaging command, so if it's running it as the player then effectively the player is sending a PM to themselves. But the placeholder text is being replaced properly, yes!

Also, I optimised/improved some of the regex patterns for the searches, I can post them once I'm done. But first, for the custom commands bit, it seems to be a map rather than a list of commands... I'm not sure how to add more.

custom-commands:
  trigger:
    regex: 'search pattern to apply to everything AFTER the prefix'
    permisssion: some.permission
    commands:
      - command 1
      - command 2
      - etc

Is trigger the name of the command group or something? (Also, typo: permisssion -> permission, one s too many)

eccentricdevotion commented 4 years ago

I was thinking of adding a /handles tell command that could be used instead of the regular /tell command to send the private message as coming from handles - seems more logical to me...

Yes, trigger is the command group name/label and can be anything [a-zA-Z0-9_]

To add a new custom command, just copy-and-paste the whole group and edit, e.g.:

custom-commands:
  trigger:
    regex: 'search pattern to apply to everything AFTER the prefix'
    permisssion: some.permission
    commands:
      - command 1
      - command 2
      - etc
  weird:
    regex: 'blah blah'
    permission: tardis.handles.use
    commands:
      - handles weird
      - handles tell %player_name% I'm sorry %player_displayname%, I'm afraid I can't do that

Yep, already caught the misspelling when it wasn't getting the permission from the config...

PrincessRTFM commented 4 years ago

Ah, okay! Quick check, [a-zA-Z0-9_] does not include -, was that a typo in the docs (on here) or is - not supported in the code?

Aaand, final two ideas:

Finally, the regex patterns I've set up. Here's just my whole file right now, you can diff it to see the exact changes I made, and Regex101 is a great site to test them against various command strings (and learn a bit about how they match things!) if you want.

# must use single quotes to wrap regular expressions
enabled: true
prefix: '^(?:hey,?\s+)?handles(?:[,!:\s]|\.\.\.+)'
reminders:
  enabled: true
  schedule: 1200
core-commands:
  craft: '\b(?:craft|build|make|create)\b.*\b(\w+)\s+(tardis\b)?(\b.*\b)'
  remind: '\bremind\s*(?:me\s+to)?\s+(.+)\s+.+(\d+)'
  say: '\bsay\s+(.+)'
  name: '\bname\b'
  time: '\btime\b'
  call: '\bcall\b'
  takeoff: '\btake\s*off\b'
  land: '\bland\b'
  hide: '\bhide\b'
  rebuild: '\brebuild\b'
  direction: '\b(?:fac(?:ing|e)|direction)\s+(\w+)'
  lights: '\b(?:lights\b.*\b(off|on)|(off|on)\b.*\blights)\b'
  power: '\b(?:power\b.*\b(off|on)|(off|on)\b.*\bpower)\b'
  travel:
    save: '\b(?:travel|go)\s+(?:to\s+)?.*?(?:saved?(?:\s+(?:location|destination|place|point))?|destination)\s+(\w+)'
    home: '\b(?:travel|go)\s+(?:to\s+)?.*?home\b'
    random: '\b(?:(?:travel|go)\s+(?:to\s+)?.*?|find\b.+)\brandom\b'
    player: '\b(?:travel|go)\s+(?:to\s+)?.*?player\s+(\w+)'
    area: '\b(?:travel|go)\s+(?:to\s+)?.*?(?:\b(\w+)\s+area|area\s+(\w+))\s*?$'
    biome: '\b(?:(?:travel|go)\s+(?:to\s+)?.*?|find\b.+)(?:\b([\w:]+)\s+biome|biome\s+(\w+))\s*?$'
    cave: '\b(?:(?:travel|go)\s+(?:to\s+)?.*?|find\b.+)\bcave\b'
    village: '\b(?:(?:travel|go)\s+(?:to\s+)?.*?|find\b.+)\bvillage\b'
  door:
    open: '\b(?:open\b.*\bdoor|door\b.*\bopen(?:ed)?)\b'
    close: '\b(?:close\b.*\bdoor|door\b.*\bclosed?)\b'
    lock: '\b(?:lock\b.*\bdoor|door\b.*\block(?:ed)?)\b'
    unlock: '\b(?:unlock\b.*\bdoor|door\b.*\bunlock(?:ed)?)\b'
  scan: '\bscan\b'
  teleport: '\bteleport\b'
  transmat: '\btransmat\s+(?:(?:me\s+)?to\s+(?:the\s+)?)?(.+)'
custom-commands:
  trigger:
    regex: 'search pattern to apply to everything AFTER the prefix'
    permission: some.permission
    commands:
      - command 1
      - command 2
      - etc
eccentricdevotion commented 4 years ago

I generally don't use a - just because they're used for lists... but valid all the same.

Can't use # for console command as it is the comment character in YAML - maybe © (copyright symbol - c for console?) or ~. Otherwise easy to check for the character and send as console instead.

I'll have a play and see if I can get capture groups to work... shouldn't be too hard as we're already doing it for the core commands.

eccentricdevotion commented 4 years ago

814b5c0 163c9df

PrincessRTFM commented 4 years ago

For the prefix, a ~ would be preferable because my keyboard (american) doesn't have a © key, but other options might include ^ (elevate the command to the console level), ! (shouted command as in "I don't care if the player can do it, run it!"), and $ (command line, need I say more?) maybe.

The - would be nice just for a name like... well, like core-commands - multiple words but with - instead of _. Obviously, you shouldn't START the name with it, the YAML parser might not like that, but any admin editing this file should know that much at least.

PrincessRTFM commented 4 years ago

Just remembered ! is used by YAML tags, so that one's out, but the others should still work as options

eccentricdevotion commented 4 years ago

My keyboard doesn't have a © key either, but it's a trivial character to type on a Mac (alt/option + G) My bash prompt is ~ % and I don't want server admins to get confused with different uses of $ (console & capture groups), ~ is too similar to -, so I'm going with the caret ^.

PrincessRTFM commented 4 years ago

All good points, and the caret is easy to use and remember!