MC-143269Nested intermediate functions are skipped when maxCommandChainLength commands are already queued
The bug
Functions can skip intermediate nested functions if maxCommandChainLength (-1) commands are already queued.
While it is generally problematic when a function stops midway due to this gamerule, this specific behavior for nested functions is even more problematic:
A >B >C D
Here the main function executes A, then a nested function containing B and C, and then D. Due to the behavior described above, it is possible that B and C are skipped, but D is executed. This can cause quite some problems when D relies on B and C having run before.
How to reproduce
- Download the attached datapack MC-143269.zip
and place it in the datapacks folder of your world - Decrease maxCommandChainLength
/gamerule maxCommandChainLength 3
- Run the function test:run_nested
/function test:run_nested
→
It does not display "Nested", but it does display "Requiring nested"
Code analysis
The following is likely outdated, please also have a look at liach's patch.
See net.minecraft.advancements.FunctionManager.execute(FunctionObject, CommandSource)
This could be solved by removing the check at the beginning and always adding the nested function to the queue.
The loop removing the queue elements could also be improved by passing
maxChainCommandsCount - executedCommandsCount - 1
to the execute method of the queued command instead of maxChainCommandsCount.
With maxChainCommandsCount = MCP: i; executedCommandsCount = MCP: j
And - 1 since executedCommandsCount is incremented afterwards.
Additionally commandQueue could be changed from a ArrayDeque to a ring buffer with the size of maxCommandChainLength, created every time a non-nested function is executed. There is for example CircularFifoQueue (Apache Commons Collections) which could be used.
Linked Issues
Attachments1
LowCreated Issue:
Nested intermediate functions are skipped when maxCommandChainLength commands are already queued
The bug
Functions can skip intermediate nested functions if maxCommandChainLength (-1) commands are already queued.
While it is generally problematic when a function stops midway due to this gamerule, this specific behavior for nested functions is even more problematic:Function exampleA >B >C DHere the main function executes A, then a nested function containing B and C, and then D. Due to the behavior described above, it is possible that B and C are skipped, but D is executed. This can cause quite some problems when D relies on B and C having run before.
How to reproduce
- Download the attached datapack [^###] and place it in the datapacks folder of your world
- Decrease maxCommandChainLength
/gamerule maxCommandChainLength 3- Run the function test:run_nested
/function test:run_nested→
Code analysis
See net.minecraft.advancements.FunctionManager.execute(FunctionObject, CommandSource)
This could be solved by removing the check at the beginning and always adding the nested function to the queue.
The loop removing the queue elements could also be improved by passingmaxChainCommandsCount - executedCommandsCount - 1to the execute method of the queued command instead of maxChainCommandsCount.
With maxChainCommandsCount = MCP: i; executedCommandsCount = MCP: j
And - 1 since executedCommandsCount is incremented afterwards.Additionally commandQueue could be changed from a ArrayDeque to a ring buffer with the size of maxCommandChainLength, created every time a non-nested function is executed. There is for example CircularFifoQueue (Apache Commons Collections) which could be used.
relates to
The bug
Functions can skip intermediate nested functions if maxCommandChainLength (-1) commands are already queued.
While it is generally problematic when a function stops midway due to this gamerule, this specific behavior for nested functions is even more problematic:Function exampleA >B >C DHere the main function executes A, then a nested function containing B and C, and then D. Due to the behavior described above, it is possible that B and C are skipped, but D is executed. This can cause quite some problems when D relies on B and C having run before.
How to reproduce
- Download the attached datapack [^###] and place it in the datapacks folder of your world
- Decrease maxCommandChainLength
/gamerule maxCommandChainLength 3- Run the function test:run_nested
/function test:run_nested→
Code analysis
See net.minecraft.advancements.FunctionManager.execute(FunctionObject, CommandSource)
This could be solved by removing the check at the beginning and always adding the nested function to the queue.
The loop removing the queue elements could also be improved by passingmaxChainCommandsCount - executedCommandsCount - 1to the execute method of the queued command instead of maxChainCommandsCount.
With maxChainCommandsCount = MCP: i; executedCommandsCount = MCP: j
And - 1 since executedCommandsCount is incremented afterwards.Additionally commandQueue could be changed from a ArrayDeque to a ring buffer with the size of maxCommandChainLength, created every time a non-nested function is executed. There is for example CircularFifoQueue (Apache Commons Collections) which could be used.
The bug
Functions can skip intermediate nested functions if maxCommandChainLength (-1) commands are already queued.
While it is generally problematic when a function stops midway due to this gamerule, this specific behavior for nested functions is even more problematic:Function exampleA >B >C DHere the main function executes A, then a nested function containing B and C, and then D. Due to the behavior described above, it is possible that B and C are skipped, but D is executed. This can cause quite some problems when D relies on B and C having run before.
How to reproduce
- Download the attached datapack MC-143269.zip
- Decrease maxCommandChainLength
/gamerule maxCommandChainLength 3- Run the function test:run_nested
/function test:run_nested→
Code analysis
See net.minecraft.advancements.FunctionManager.execute(FunctionObject, CommandSource)
This could be solved by removing the check at the beginning and always adding the nested function to the queue.
The loop removing the queue elements could also be improved by passingmaxChainCommandsCount - executedCommandsCount - 1to the execute method of the queued command instead of maxChainCommandsCount.
With maxChainCommandsCount = MCP: i; executedCommandsCount = MCP: j
And - 1 since executedCommandsCount is incremented afterwards.Additionally commandQueue could be changed from a ArrayDeque to a ring buffer with the size of maxCommandChainLength, created every time a non-nested function is executed. There is for example CircularFifoQueue (Apache Commons Collections) which could be used.
![[Mojang] Nathan Adams](../../../data/assets/gravatar/dinnerbone.png){kind=small}
![[Mojang] Adrian Östergård](../../../data/assets/gravatar/nighter.jpeg){kind=small}
![[Mojang] slicedlime](../../../data/assets/gravatar/slicedlime.jpeg){kind=small}
The bug
Functions can skip intermediate nested functions if maxCommandChainLength (-1) commands are already queued.
While it is generally problematic when a function stops midway due to this gamerule, this specific behavior for nested functions is even more problematic:Function exampleA >B >C DHere the main function executes A, then a nested function containing B and C, and then D. Due to the behavior described above, it is possible that B and C are skipped, but D is executed. This can cause quite some problems when D relies on B and C having run before.
How to reproduce
- Download the attached datapack MC-143269.zip
- Decrease maxCommandChainLength
/gamerule maxCommandChainLength 3- Run the function test:run_nested
/function test:run_nested→
Code analysis
The following is likely outdated, please also have a look at liach's patch.
See net.minecraft.advancements.FunctionManager.execute(FunctionObject, CommandSource)
This could be solved by removing the check at the beginning and always adding the nested function to the queue.
The loop removing the queue elements could also be improved by passingmaxChainCommandsCount - executedCommandsCount - 1to the execute method of the queued command instead of maxChainCommandsCount.
With maxChainCommandsCount = MCP: i; executedCommandsCount = MCP: j
And - 1 since executedCommandsCount is incremented afterwards.Additionally commandQueue could be changed from a ArrayDeque to a ring buffer with the size of maxCommandChainLength, created every time a non-nested function is executed. There is for example CircularFifoQueue (Apache Commons Collections) which could be used.
![[Helper] Jack McKalling](../../../data/avatars/users/92859.png){kind=small}
![[Mojang] Bartosz Bok](../../../data/avatars/users/10122.svg){kind=small}
[Mojang] Bartosz Bok
As the title says, Function command returns wrong number of commands executed for embedded functions.
As far as I see, this wrong number of command is considered by the maxCommandChainLength game rule.
A data pack to replicate this issue is available in the attachment section here. (dp.zip)
Example: Given two functions:
liach:output
function liach:output/do
liach:output/do
say Expected to have 2 commands ran in the function liach:output
Execution result of liach:output (Looks wrong)
[liach] Expected to have 2 commands ran in the function liach:output
Executed 3 commands from function 'liach:output'
Execution result of liach:output/do (Looks right)
[liach] Expected to have 2 commands ran in the function liach:output
Executed 1 commands from function 'liach:output/do'
What I expect for execution result of liach:output
[liach] Expected to have 2 commands ran in the function liach:output
Executed 2 commands from function 'liach:output'
The data pack containing these 2 functions is attached below (note that there is a few unrelated functions for testing other bugs)
I am working on a solution that fixes this bug, MC-143266, MC-143269, and MC-126946 at the same time. Will update when I get progress.
I believe this should be fixed with
MC-126946together, as the fix offered here is based on a wrong data structure the code currently use. Will post an updated solution soon.Instead of using an FIFO queue, the fix should be incorporated to that for
MC-126946.A sample patched version is available at
https://github.com/liachmodded/MC-126946-128565/blob/2cbe0650ebe385027e484e7496fc63ebba690cad/src/main/java/com/github/liachmodded/datapacks/mixin/CommandFunctionManagerMixin.java#L99-L117