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 example
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
and place it in the
datapacks
folder of your worldDecrease
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 @unknown'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|https://commons.apache.org/proper/commons-collections/javadocs/api-4.2/org/apache/commons/collections4/queue/CircularFifoQueue.html]
(Apache Commons Collections) which could be used.
Linked issues
relates to 1
Attachments
Comments 2
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
I believe this should be fixed with MC-126946 together, as the fix offered here is based on a wrong data structure the code currently use. Will post an updated solution soon.