
Bug: Scarpet `run('command')` function does not return any values
Oxtaly opened this issue ยท 3 comments
Description
The run('command')
function never (except in a syntax exception error) returns the command's values, and always returns [0, [], null]
Evidence/steps to reproduce:
Test 1: /script run run('execute if entity @s')
Expected output: [1, [Test passed, count: 1], null]
(Ran with minecraft 1.20.1, fabric 0.16.9, carpet v1.4.112+v230608)
Observed output: [0. [], null]
(Ran with minecraft 1.21.4, fabric 0.16.9, carpet v1.4.161+v241203)
Test 2: /script run run('execute unless entity @s')
Expected output: [0, [], Test failed, count: 1]
(Ran with minecraft 1.20.1, fabric 0.16.9, carpet v1.4.112+v230608)
Observed output: [0. [], null]
(Ran with minecraft 1.21.4, fabric 0.16.9, carpet v1.4.161+v241203)
Guessed Source
Thank you to @dmunozv04 for finding the source here;
We believe that due to the new way commands are handled, the current implementation of the run('command')
function returns too early before the command is ever executed, provided it isn't a syntax error, which does give the correct output (see below)
/script run run ('exec')
Attached is an image of the implementation with an additional oversight highlighted, where the command success value is always set to 0, regardless of the actual command execution
Ok, I did some digging and figured out the problem
The command execution changed so that when you run a command while you are handling a command, the requested command gets put on the pile allowing the current command to finish.
Its a design choice but a way cleaner path to execute nested commands.
This impacts situations like
/script run run('setblock ....')
cause setblock will be run in a context of running the 'script' command
It has nothing to do with threading.
The issue can be mitigated by not running commands from commands, for instance this works:
/script run schedule(1, _() -> print(player(), run('/fill 200 100 200 201 101 201 stone')))
as well as calling run()
in tick
event or anywhere else but /script run/invoke
Hey there, my deepest apologies for the delayed response;
I had missed the fact that this bug would only occur if it was called from another command, so this problem scope is more limited than I first thought, thank you for the added explanation and details;
I believe keeping run_cb()
(possibly renamed to run_async()
as you mentioned?) could still be good for usages in command ran contexts (notably custom script commands) without the need for a workaround or places where retrieving the first error message is necessary (although that would be quite a rare edge case)
(edited, had not seen the new commits, thank you for fixing the return code issue and updating the documentation)
I've reopened the pull request (#2036) with run_async(expr, callback)
+ documentation and small documentation changes/precisions for run(expr)
, but if run_async
isn't something you're looking to add into scarpet feel free to close both this issue and PR ๐