CortexPE / Commando

A Command Framework virion for PocketMine-MP
GNU Lesser General Public License v3.0
70 stars 53 forks source link

Issue parsing BlockPositionArgument + RawStringArgument #13

Open inxomnyaa opened 5 years ago

inxomnyaa commented 5 years ago

My command:

    /**
     * This is where all the arguments, permissions, sub-commands, etc would be registered
     * @throws \CortexPE\Commando\exception\ArgumentOrderException
     */
    protected function prepare(): void
    {
        $this->registerArgument(0, new BlockPositionArgument("position", false));
        $this->registerArgument(1, new RawStringArgument("block", false));
    }

    /**
     * @param CommandSender $sender
     * @param string $aliasUsed
     * @param BaseArgument[] $args
     */
    public function onRun(CommandSender $sender, string $aliasUsed, array $args): void
    {
        if (isset($args["position"]) && isset($args["block"])) {
            /**
             * @var Vector3 $position
             * @var string $block
             */
            $position = $args["position"];
            $block = $args["block"];
            $sender->sendMessage(strval(extract($args)));
            $sender->sendMessage(strval($position) . strval($block));
        } else {
            $this->sendUsage();
        }
    }

With some tests, i got these results:

block
[12:17:44] [Server thread/INFO]: Usage: /block <position:x y z> <block:string>
block 1
[12:17:45] [Server thread/INFO]: Invalid value '1' for argument #1
block 1 1
[12:17:46] [Server thread/INFO]: Invalid value '1' for argument #1
block 1 1 1
[12:17:49] [Server thread/INFO]: 2
[12:17:49] [Server thread/INFO]: Vector3(x=1,y=1,z=1)
block 1 1 1 tnt
[12:18:03] [Server thread/INFO]: 2
[12:18:03] [Server thread/INFO]: Vector3(x=1,y=1,z=1)tnt

IMO block 1 1 should say

Invalid value '1 1' for argument #1

block 1 1 1 Should still fail, since $block is not optional. isset returns true though. block 1 1 1 tnt is the only correct syntax and correctly parsed

CortexPE commented 5 years ago

That's because you can register multiple types of arguments in one position of the argument list... And, the length can vary.

So if it tries to parse it, and gives up, it only gives back the original value because there can be many different conversions.

The reason why you can register multiple argument types in a single position is for conversion... Which is used for my 'Hierarchy' plugin. You can use both Role or Member in one argument.

On Wed, Jun 5, 2019, 6:28 PM XenialDan notifications@github.com wrote:

My command:

/**
 * This is where all the arguments, permissions, sub-commands, etc would be registered
 * @throws \CortexPE\Commando\exception\ArgumentOrderException
 */
protected function prepare(): void
{
    $this->registerArgument(0, new BlockPositionArgument("position", false));
    $this->registerArgument(1, new RawStringArgument("block", false));
}

/**
 * @param CommandSender $sender
 * @param string $aliasUsed
 * @param BaseArgument[] $args
 */
public function onRun(CommandSender $sender, string $aliasUsed, array $args): void
{
    if (isset($args["position"]) && isset($args["block"])) {
        /**
         * @var string $position
         * @var Vector3 $block
         */
        $position = $args["position"];
        $block = $args["block"];
        $sender->sendMessage(strval(extract($args)));
        $sender->sendMessage(strval($position) . strval($block));
    } else {
        $this->sendUsage();
    }
}

With some tests, i got these results:

block [12:17:44] [Server thread/INFO]: Usage: /block block 1 1 [12:17:46] [Server thread/INFO]: Invalid value '1' for argument #1 block 1 1 1

block 1 1 1 tnt

IMO block 1 1 should say

Invalid value '1 1' for argument #1 block 1 1 1 Should still fail, since $block is not optional. isset returns true though. block 1 1 1 tnt is the only correct syntax and correctly parsed

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CortexPE/Commando/issues/13?email_source=notifications&email_token=AEHQQFB3SCGO34B6UPH7CRDPY6ILLA5CNFSM4HTUXXOKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GXXPTGQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AEHQQFGYAW45ZPB25UAV3HTPY6ILLANCNFSM4HTUXXOA .

inxomnyaa commented 5 years ago

But that was not the question/issue at all.

inxomnyaa commented 5 years ago

I realized that !empty($args["block"]) is more reliable in RawStringArgument, as for some reason it allows empty strings

CortexPE commented 5 years ago

It doesn't give back '1 1' because the original value was '1'

On Thu, Jun 6, 2019, 6:36 PM XenialDan notifications@github.com wrote:

But that was not the question/issue at all.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/CortexPE/Commando/issues/13?email_source=notifications&email_token=AEHQQFGELQI23STT3MEKNS3PZDSB5A5CNFSM4HTUXXOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXCOFDI#issuecomment-499442317, or mute the thread https://github.com/notifications/unsubscribe-auth/AEHQQFDUSIBD7P2TNLYREULPZDSB5ANCNFSM4HTUXXOA .

CortexPE commented 5 years ago

It doesn't have any verification for the given input because it is indeed, "raw".

On Thu, Jun 6, 2019, 6:49 PM XenialDan notifications@github.com wrote:

I realized that !empty($args["block"]) is more reliable in RawStringArgument, as for some reason it allows empty strings

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/CortexPE/Commando/issues/13?email_source=notifications&email_token=AEHQQFBKVSZKDLK6AUDOWYDPZDTUPA5CNFSM4HTUXXOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXCPCJQ#issuecomment-499446054, or mute the thread https://github.com/notifications/unsubscribe-auth/AEHQQFA7QUTWP5PHYWJCNLDPZDTUPANCNFSM4HTUXXOA .

lukeeey commented 5 years ago

@CortexPE TextArgument also accepts no input which isnt really ideal