Authmereloaded: forceCommands redesign & fix?

[MithrandirCraft]

I think the forceCommand functions could benefit from some fixing and changes, just like the "ForceSpawnOnTheseWorlds" function, as I mention on #256

I've been trying to set a command to be executed when the player logs in, so that it opens the shop on player's faces after they log in:

(also tried with "buy" without including /)

None of these two ways have worked.
There are also the other command forcing configurations, I can't really say if they work or not, but "forceCommands" & "forceCommands" as console aren't really clear as to when the command is forced. I would like to know.

forceCommands: []
forceCommandsAsConsole: []
forceRegisterCommands: []
forceRegisterCommandsAsConsole: []

---

Spigot version: 1.8.8
Authme version: 5.2-SNAPSHOT-b596

[DNx5]

maybe

forceCommands:
  Join: []
  Login: []
  etc...

or put that on new .yml file.

[MithrandirCraft]

That would look much better n_n

[sgdc3]

Another file would be better ;) +1

[sgdc3]

Concept:

# Basic usage
onFirstJoin:
    commands: []
    commandsAsConsole:
        - "/say Welcome to %player_displayname%!"
onJoin:
    commands: []
    commandsAsConsole:
        - "/pex user %player_name% group set unlogged"
onRegister:
    commands: []
    commandsAsConsole:
        - "/pex user %player_name% group set newbie"
onLogin:
    commands: []
    commandsAsConsole:
        - "/pex user %player_name% group set %player_group%" # (Stored in MySql)

# Advanced usage
onJoin:
    commands: []
    commandsAsConsole:
        - "for %loop_var1% in %player_currentgroups% > /pex user %player_name% group remove %loop_var1%"
        - "/pex user %player_name% group set unlogged"
onLogin:
    commands: []
    commandsAsConsole:
        - "for %loop_var1% in %player_currentgroups% > /pex user %player_name% group remove %loop_var1%"
        - "for %loop_var1% in %player_groups% > /pex user %player_name% group add %loop_var1%"

[timvisee]

Sounds like a cool feature. But what about a more object oriented notation. That eliminates the need of nodes such as commandsAsConsole which is quite long.

commands:
  firstJoin:
    myCommand:
      command: "/say Running my command!"
      as: CONSOLE
    myBuyCommand:
      command: "/buy"
      as: PLAYER
    welcomeMessage:
      command: "/say Welcome %player_displayname%"
      as: PLAYER
  join: []
  register: []

First, I've called the based node commands, this should be called something different. actions might be better, especially later on when more features are added.

So, to explain what I've done: I've branched out all events on the commands base node. That base node is just to keep the configuration clean, we don't want to randomly place all kinds of event nodes in the configuration file. All event branches are of course, optional, thus the commands base could be left blank with commands: {}.
I've left the on... part out, they're all events anyway, simply to make the event names shorter and better readable.

Each event branch, contains command objects. These objects contain command-specific configurations. And again, a branch could be left blank using firstJoin: {} (but in that case, the whole branch could be left out anyways). Each command object has a name, which is simply used for the administrator to distinguish the commands. Take a look at the example above, where I've used the myCommand, myBuyCommand and welcomeMessage names. One drawback is however, that these names should be unique because YAML needs to be able to select the command object by it's node key.
Spaces in the names are allowed, but that would look kinda messy:

commands:
  firstJoin:
    'my command':
      command: "/kill" 

The command objects should allow various configurable properties. The command: node should be required, since that's used to define what command is being executed. It might be useful to allow string lists as value to allow multiple commands to be executed. Like @sgdc3 's example, variables like %player_displayname% to allow more dynamic command execution. Also, I'd prefer to use dot separation since that's used by YAML and Java too, just to be more consistent. (%player.displayname%).
I've also included the as: node in the command object. This would be used to define what the command is executed as. PLAYER and CONSOLE or SERVER would define whether the command should be executed by the current player, or by the console. I think it would be best to make this optional, and default to CONSOLE.

The good thing about this setup is that it's easily extendable. You don't have to use akwardly large strings with weird configurations to support new features. It allows you to add new command-object properties later on if needed. New command executors such as ALL_PLAYERS could easily be implemented too. Adding specific world support as mentioned above could be done by adding a optional worlds property. Adding a type property would even make it possible to call functions on plugin API's instead of using commands. Such configuration could look similar to this:

commands:
  firstJoin:
    itsMyPayDay:
      type: vault
      action: ADD_MONEY
      amount: 10000

Of course, Vault allows you to configure the base amount of money anyways. This is just a simple example, of a possible future feature.

It's quite a long message, and I'm bad at explaining things, I hope it's clear ^^.
Please let me know what you guys think!

[sgdc3]

@timvisee You're right, what do you think about the loop idea?

[ljacqu]

I like your version @timvisee in that each command is represented as its own sub-branch, which makes it easier to extend it later with more properties, like worlds (as you've mentioned).

Each command having its own name might be initially confusing to the user (where do those names come from? what names can i use?) but I can't think of anything better. It also has the obvious advantages that it provides a clean structure of the config. Additionally, if necessary for error handling we can reference commands by their name.

One thing; personally, I liked the on in "onJoin" etc. as it really conveys the notion that these commands will be executed when that event occurs. The "on" prefix is a common naming scheme when dealing with events so maybe we could revisit the idea of the prefix? :)

[timvisee]

@sgdc3 Oh yes, of course, forgot to mention that. I really like the idea, it allows administration to do so much more when needed.

About the implementation. Your example shows some custom for loops, in the same string. The problem with this is that you'd need to build your own interpreter, which executes for-loop-like-statements, and of course, the command itself. Like I've sorta mentioned in my previous message, I don't like long unreadable strings, which you'll be creating this way. It tickles my OCD haha. Besides, it requires the administrator to learn yet another 'language'.

A different solution would be to kneeling a similar, object oriented way, for commands too. The problem with that solution is though, that you'll be creating massive objects. Those are hard to read, manage, and understand. We haven't even talked about the actual implementation of this solution yet. How would for-loops be constructed this way? It's probably too difficult.

Now, what about LUA? What if administrators are allowed to use executable LUA code for these commands. I don't really like LUA myself to be honest, but it has many pro's in comparison with the other solutions I gave. Just to name two; it's easy to implement in the project, and it already contains all the different programming statements you'd want without the need to implement these yourself.

What do you guys think about LUA, to allow basic programming for complex situations?

[timvisee]

@ljacqu Good to hear!

First, about the on prefix: I was just giving a suggestion. And you're right that on implies that you're working with an event like system. I'm sure that the prefix is beneficial for administrators that don't often use features like this, so, yes: implement the prefix!

Indeed, the command name can be a little weird since it isn't really visible to the user, it's just there to structure the configuration. There's one other implementation I can think of, which is using object lists. It's an official YAML feature, and works similar to the string lists in a previous example, but it uses objects instead. Now, to give an example, check out this configuration:

commands:
    onFirstJoin:
        - command: "/say Welcome ..." 
          as: CONSOLE
        - command: "/spawn" 
          as: PLAYER

With a configuration like this, you can fetch a list of objects from the commands.onFirstJoin node. This eliminates the need of command name, but it looks worse and is hard to work with (programmably and when configuring). That's why I think the 'named' configuration is our best option. Oh, and the error messages with command names, brilliant! I've never really thought about it that way.

[ljacqu]

@timvisee I'm very much in favor of named commands—now even more so :smile: Just thinking aloud that it might be a little confusing for a user on first sight. Those object lists... _shudders_ :stuck_out_tongue_closed_eyes:

[sgdc3]

What do you guys think about LUA, to allow basic programming for complex situations?

At this point we can just add Skript support. :)

[sgdc3]

or JavaScript idk

[timvisee]

@sgdc3 Skript however, is less commonly used, isn't it? Also, does it allow all the necessary features to support command-scripting?

I don't think JavaScript is too useful. It requires a runtime like V8 which is huge, bulky, and hard to implement, especially in Java (as far as I know).

[sgdc3]

@timvisee skript is used a lot but his development seems stopped.

[sgdc3]

Not official but still active: https://github.com/bensku/Skript

[timvisee]

@sgdc3 Hmm. We shouldn't use libraries that aren't actively maintaned anymore. That will end up in us having to do much more work fixing all issues in the library.

If the fork you've linked is actively maintaned, and will be the upcoming year (or whatever), sure. That would be an option!

[foxi69]

would be nice really :D

[ljacqu]

Hi @MithrandirCraft, please verify :wink:

[MithrandirCraft]

will do ASAP, allthough I gave up on the idea of using this feature, I may use it in the future and I owe you guys some testing.