The various kinds of commands

[xoft]

I'd like to open a discussion about implementing the various kinds of commands. Right now I'm aware of three distinct kinds that each has a slightly different behavior:
- console commands (given by the server owner directly on the server's console screen)
- in-game commands (given by any player connected to the server)
- command block commands (executed by command blocks in the game)

The console commands have no connection to any player or world. The in-game commands are usually bound to a specific player or world. The command block commands are somewhere in between - they are bound to a world, but don't have any player connection.

Cuberite currently implements only the first two kinds. They have separate API functions that are used to register them, and have different callback signatures; it is possible to register a console command without its in-game counterpart and vice versa. Now the question is, how do we add the third kind, command block commands? Do we add third set of API for it, or do we try to bend one of the existing ones towards it?

One more thought about the command handlers. With the advent of InfoReg.lua usage, they command handlers in the plugins are actually slowly drifting towards a common signature. The console commands are currently (erroneously) called with an extra parameter that is always nil, in the place where the in-game command handlers have their cPlayer parameter. This was an accident in my initial implementation, but has gone too long unnoticed and it's kinda late to fix it. It also kinda makes sense - there's no cPlayer object for the command, so just send a nil instead. The command block commands could re-use this and send the originating command block instead of the cPlayer object.

Return values of the command handlers are currently slightly different as well. In-game commands return only status - was it executed or not? Console commands return status, and an optional message to output to the console as the command's result. Command blocks' command handlers should also output some form of a result string. Should we unify this behavior somehow?

Side note regarding console command handlers. There is a growing need of an "semi-asynchronous" command handling - the command handler itself terminates, but it needs to process something on the background (typically wait for a chunk to load) and then finalize itself. A similar situation could develop with in-game commands (e. g. teleport command waiting for the chunks to load before teleporting). A solution has been proposed to this - add another parameter that handles the command output and termination; as long as the command doesn't explicitly terminate, more output can be sent to the object.

Personally, I'm inclined to take this opportunity (adding another command kind) and unify the callbacks' signatures, as well as add the CommandOutput object as a parameter. I'd keep the registration API separate, though.

function HandleIGCmdHello(a_Split, a_Player, a_EntireCmd, a_CommandOutput)
  -- In-game command handler, a_Player is a cPlayer object
  return true, "SomeMessage"  -- SomeMessage is sent to the player; nothing gets sent if an empty string or nil is returned
end

function HandleConCmdHello(a_Split, a_Nil, a_EntireCmd, a_CommandOutput)
  -- Console command handler, a_Nil is always nil
  return true, "SomeMessage"  -- SomeMessage is output to the console
end

function HandleCBCmdHello(a_Split, a_CommandBlockEntity, a_EntireCmd, a_CommandOutput)
  -- Command block command handler, a_CommandBlockEntity is the cCommandBlockEntity executing the command
  return true, "SomeMessage"  -- SomeMessage is set as the command's output (shown in the UI)
end

If the command handler returns a string as the second parameter, the command is assumed to have terminated, the server automatically sets a_CommandOutput as terminated. If only the bool value is returned, the command output stays "open" and the plugin is expected to call a_CommandOutput:Terminate() at some point.

Thoughts?

[NiLSPACE]

I'm not sure what you mean with the CommandOutput parameter. Could you elaborate? If I understand it correctly you plan to use it to send a message to the player/console/command block when Terminate is called? Can I add information when, by your example, the chunk is loaded? How would that work? Would it look a little like this?:

function HandleCertainCommand(a_Split, a_Player, a_EntireCmd, a_CommandOutput)
	local function OnChunkLoaded(a_ChunkX, a_ChunkZ)
		a_CommandOutput:AddToMsg(a_ChunkX .. " " .. a_ChunkZ .. "was loaded")
		a_CommandOutput:Terminate()
	end
	
	-- Load the chunk the player is standing in
	
	return false, "Chunk "
end

It does make sense to have all type of command handlers return the message to be send. I'm all for it

[bibo38]

What about the way, how Bukkit handles the commands.
Bukkit uses an abstract class called CommandSender, which is implemented by every entity, which can send commands.
It has some basic commands(https://hub.spigotmc.org/javadocs/bukkit...ender.html) to allow sending messages back to the command sender and to check permissions of the command sender. These are then properly overloaded(console has all permissions, command block messages are lost...) and send to the Command handeling routine.

The effect is, that you only have one command handeling routine, which can decide, if it threads console commands different than player commands:

Code:

function HandleCommand(a_CommandSender, a_Split, a_EntireCmd)
    if(a_CommandSender:GetType() != SenderType:Player && a_Split.length == 0) then
        -- No playername to kill and no player has send this command
        a_CommandSender:sendMessage("I don't know, who I should kill...")
        return
    end
    -- Kill the given player or the player itself, if it is a player
end

The nice thing about this is, that a lot of commands don't need to have a player. Also most command block commands behave exactly, as if they were executed on a command block(only certain commands differ, like /ban or /op).
Also you can write little wrapper functions, which wrap a command handler and check for a player executing a command.

P.S. @xoft Have you thought about the Mincarts with command blocks inside ? (see http://minecraft.gamepedia.com/Command_block_minecart)

[xoft]

I don't like Bukkit's way of putting all the commands together. If one plugin registers a "test" console command, another plugin cannot register a "test" in-game command (which is perfectly possible now in Cuberite).

Minecarts with command blocks could use commandblock registrations and call the callbacks with a_CommandBlockEntity set to the minecart-with-CB. Not much pain.

@NiLSPACE That's more or less what I had in mind - the CommandOutput object is there so that a handler may add any output up until it calls Terminate:

--- Lists all entities in a given chunk
function HandleConsoleEnumChunkEntities(a_Split, a_Player, a_EntireCmd, a_CommandOutput)
  local worldName = a_Split[2]
  local chunkX = tonumber(a_Split[3])
  local chunkZ = tonumber(a_Split[4])
  
  local world = cRoot:Get():GetWorld(worldName)
  world:ChunkStay({{chunkX, chunkZ}},
    nil,  -- ChunkAvailable
    function ()  -- AllChunksReady
      -- The chunk has been loaded, enum the entities:
      world:ForEachEntityInChunk(chunkX, chunkZ,
        function (a_Entity)
          a_CommandOutput.Add(string.format("Entity %s at {%d, %d, %d}", 
            a_Entity:GetClass(), a_Entity:GetPosX(), a_Entity:GetPosY(), a_Entity:GetPosZ())
          )
        end
      )
      -- All entities have been listed, terminate the output:
      a_CommandOutput.Terminate()
    end
  )

  return true  -- Must not return a message - that would terminate the CommandOutput immediately
end

[bibo38]

Wouldn't overloading commands be confusing?
But anyways, you can also create a register method, where you can register the command for specific types only.
The main goal of this attempt is to avoid code deduplication. Look at the /list command in the Core plugin.
It is defined in https://github.com/mc-server/Core/blob/m...le.lua#L86 and in https://github.com/mc-server/Core/blob/m...ist.lua#L1.
The result is: Nearly 100% copied code(except the command output) and in order to fix a command, you have to copy the code and slightly modify it.

Also the a_CommandOutput is too static in my eyes. Think of a command, which informs you, when a player mines a diamond block. You want to send single messages and not the whole message block at once.

P.S.: What about of giving permissions to command blocks? This would give the Server owners more control about the command blocks. Think of a command block, which can execute a /save-all on a specific action.