mojira.dev
MC-262027

Result consumer created by "/execute store" is invoked for every command within a function

The bug

/execute store is a command that runs subsequent commands in a new command environment with a result consumer that consumes command results. This result consumer is carried over across /function, and is invoked every time a command within the function is executed. This behavior is natural when considering that other elements such as positions and entities are similarly carried over. However, this behavior introduces overhead when obtaining the result of /function with /execute store. In particular, when obtaining the result of a recursive function with /execute store, the cost per command is quadratic with respect to the recursion depth.

How to reproduce

Simple case

  1. Run

    /function mc-262027:simple-a

data/mc-262027/functions/simple-a.mcfunction

data modify storage mc-262027: simple set value 0
execute store result storage mc-262027: simple int 1 run function mc-262027:simple-b

data/mc-262027/functions/simple-b.mcfunction

# This command prints 0.
tellraw @a {"nbt": "simple", "storage": "mc-262027:"}

say mc-262027:

# This command prints 1 because the result consumer was applied to the result value of `/say <message>` (1).
tellraw @a {"nbt": "simple", "storage": "mc-262027:"}

Recursive case

In the case of recursion, reproduction steps become a bit more complicated. This is because result consumers perform a set operation, making it difficult to observe whether they have been executed multiple times.

  1. Run

    /gamerule maxCommandChainLength 12
  2. Run

    /function mc-262027:recursive-a
  3. Run

    /tellraw @a {"nbt": "recursive", "storage": "mc-262027:"}

    → formatted output:

    [
      {_: 0},
      {_: 0}, {_: 0},
      {_: 0}, {_: 0}, {_: 0},
      {_: 0}, {_: 0}, {_: 0}, {_: 0},
      {_: 0}, {_: 0}, {_: 0}, {_: 0}, {_: 0},
    ]

    From the above output, we can see that the function mc-262027:recursive-b has recursed 5 times, and the result consumers have been invoked a total of 15 times (= (5 * (5 + 1)) / 2), which is quadratic.

data/mc-262027/functions/recursive-a.mcfunction

data remove storage mc-262027: recursive
function mc-262027:recursive-b

data/mc-262027/functions/recursive-b.mcfunction

# The NBT path [{_: 1}] adds {_: 1} to the target list tag if it is not present.
# Then, the NBT path _ sets 0 to the _ in all compound tags within that list tag.
# As a result, the list tag loses {_: 1} once again.
# By repeating this process, elements can be continuously added to the list tag.
# Note that for this reproduction function, the cost is **quartic** due to searching within the list tag.
execute store result storage mc-262027: recursive[{_: 1}]._ int 0 run function mc-262027:recursive-b

Linked issues

Attachments

Comments 4

As a personal opinion, this behavior makes me hesitant to get the return value by /return <value>, which was added in 23w16a, using /execute store result ... run function ....

In short, “/execute store ... run function ... ” stores return values of all commands in the function. Once a command in the function is executed, its return value is stored. At the end of execution, the return value of the /function command is then stored.
This does make sense, and pack creators often use this feature in their data packs. So do I. But as /return is added, there is a problem: we can't get the return value of /return command in a function without storing the values of all commands in the function.

This behavior is in my opinion very awkward because it contradicts the initial idea of how functions should work (especially because /return exists now). A function (usually) returns one value at the end of execution. /execute store result ... run function ... is expected to store that return value inside of a score/NBT path/etc., but instead what it does is store all the different return values from the individual commands (and, in fact, from the original function call which returns 0) one after the other inside of the store location and the return values coincidentally match because the last command was the return command and overrode the previous value. This is also the reason /return run function doesn't work (MC-264595): The original function call returns 0 and the return run result consumer immediately aborts the function before actually running a single command. And even if the original function call wouldn't activate the result consumer, the first command in the function would and only one command would be executed.

How to fix?

To fix this issue, the result consumer would need to be detached when a function call happens and replaced by a return value consumer. When the function then returns (or finishes maybe), the return value consumer should be executed. One issue might persist, specifically, the result consumer would also need to be removed from the original command context as the function call happens and I am not sure if the command system is currently equipped for that (I think that would require a redirect in the current system).

Should have been fixed in 23w41a.

intsuc

(Unassigned)

Plausible

Platform

Normal

Commands, Performance

1.19.4, 23w16a, 23w17a, 23w18a, 1.20 Pre-release 7, 1.20, 1.20.1, 23w31a, 23w32a, 1.20.2

23w41a

Retrieved