Execution order is reversed when calling a function in some cases

I have encounterd the same effect while trying to make a datapack which sorts the players by distance to a given entity.

Confirmed in 1.14 Pre-Release 2 with the given armor stand setup.

A possible fix (Based on FabricMC yarn "1.14 Pre-Release 2+build.1"): (re.class) (All obfuscated names apply for 1.14 Pre-Release 2)

private final ArrayDeque<Entry> waitlist= new ArrayDeque<>(); // liach: Add new field in class for waitlist

// Main method
public int execute(CommandFunction commandFunction_1, ServerCommandSource serverCommandSource_1) {
   int int_1 = this.getMaxCommandChainLength();
   if (this.field_13411) {
      if (this.chain.size() < int_1) {
         // this.chain.addFirst(new CommandFunctionManager.Entry(this, serverCommandSource_1, new CommandFunction.FunctionElement(commandFunction_1)));
        this.waitlist.add(new CommandFunctionManager.Entry(this, serverCommandSource_1, new CommandFunction.FunctionElement(commandFunction_1))); // liach: Add to waitlist instead
      }

      return 0;
   } else {
      try {
         this.field_13411 = true;
         int int_2 = 0;
         CommandFunction.Element[] commandFunction$Elements_1 = commandFunction_1.getElements();

         int int_3;
         for(int_3 = commandFunction$Elements_1.length - 1; int_3 >= 0; --int_3) {
            this.chain.push(new CommandFunctionManager.Entry(this, serverCommandSource_1, commandFunction$Elements_1[int_3]));
         }

         while(!this.chain.isEmpty()) {
            try {
               CommandFunctionManager.Entry commandFunctionManager$Entry_1 = (CommandFunctionManager.Entry)this.chain.removeFirst();
               this.server.getProfiler().push(commandFunctionManager$Entry_1::toString);
               this.waitlist.clear(); // liach: (Optional) clear the waitlist before another entry
               commandFunctionManager$Entry_1.execute(this.chain, int_1);
               // liach start - Add waitlist finally; we don't reverse the order of entries in the wait list
               while (!this.waitlist.isEmpty()) {
                 this.chain.addFirst(this.waitlist.removeLast());
               }
               // liach end
            } finally {
               this.server.getProfiler().pop();
            }

            ++int_2;
            if (int_2 >= int_1) {
               int_3 = int_2;
               return int_3;
            }
         }

         int_3 = int_2;
         return int_3;
      } finally {
         this.chain.clear();
         this.field_13411 = false;
      }
   }
}

I have tested this fix with mixin and it appears to work.

This fix can resolve this issue; however, because of the addition of such a waitlist, in long term, Mojang can consider reworking command function object type and even removing the custom execution logic present within method ca$d#a. That should simplify command function code by a lot and improve its maintainability significantly.

Addition: by "simplifying command function code", I mean all those "addFirst" to the chain deque should be replaced by call to "CommandFunctionManager#execute", such as that in the early part of "CommandFunctionManager#execute" and the one in "CommandFunction.FunctionElement#execute"; the waitlist should be passed to the elements in the future as well (just add to waitlist, and check chain length limit with the length of both list)

Now, I have added another optimization (removing unnecessary boolean) and changed the command function entries to add to the waitlist instead of to the stack itself. This should make the code a bit easier to understand in the future.

See this (the license permits use of the code if you want):
https://github.com/liachmodded/MC-126946-128565/blob/master/src/main/java/com/github/liachmodded/datapacks/mixin/FunctionElementMixin.java
https://github.com/liachmodded/MC-126946-128565/blob/master/src/main/java/com/github/liachmodded/datapacks/mixin/CommandFunctionManagerMixin.java

Should have resolved this issue in the best sense.

With my test project, both MC-126946 and MC-128565 is fixed; the dp.zip data pack has a function "liach:foo" to test this (need to run "liach:armorstand" to set up)

Confirmed for 1.14 Pre-Release 3.

Confirmed with 1.14 Pre-Release 4. A more comprehensive fix is updated at https://github.com/liachmodded/MC-126946-128565/blob/2cbe0650ebe385027e484e7496fc63ebba690cad/src/main/java/com/github/liachmodded/datapacks/mixin/CommandFunctionManagerMixin.java#L70

Thank you @boq for your fix!