refactor command - #309
Conversation
There was a problem hiding this comment.
duplication: the Source/Argument + ResolvedX + resolve() scaffold is copy-pasted in 5 files (clear.rs:37, fly.rs:56, gamemode.rs:57, kill.rs:32, teleport.rs:72, with clear/fly identical apart from the name). that's ~150 lines that one shared helper on SteelCommandContext (execution/runtime.rs:373) would replace
public api: the public CommandNode (api.rs:154) didn't get the new builder methods, so extension authors are left on exactly the chained .then() shape this pr set out to fix. forwarding both is a few mechanical lines
| let destination = required_coordinates(context, "location")?; | ||
| let resolved = variant.resolve(context)?; | ||
| let targets = resolved.targets.resolve(context)?; |
There was a problem hiding this comment.
bug: this resolves location/rotation/facing before the targets selector. vanilla (TeleportCommand.java:84-103) evaluates targets first, and base matched that, so the wrong error surfaces when multiple args are bad: /tp @e[type=zombie] 0 64 0 facing entity @a with no zombies + 2 players now says "Only one entity is allowed" instead of "No entity was found".
root cause is that ResolvedPositionTeleport.targets holds an unresolved selector despite the name. making it carry real resolved targets (resolved first, before the facing/rotation reads) fixes both the name and the ordering.
There was a problem hiding this comment.
I think I have fix it correctly not sure
add more then function to reduce the number of tabulation.
reduce the number of function in command