[MC-117428] /function with /execute "An unknown error occurred while attempting to perform this command" Created: 13/May/17  Updated: 18/May/17  Resolved: 18/May/17

Status: Resolved
Project: Minecraft: Java Edition
Component/s: None
Affects Version/s: Minecraft 1.12 Pre-Release 1, Minecraft 1.12 Pre-Release 2, Minecraft 1.12 Pre-Release 3
Fix Version/s: Minecraft 1.12 Pre-Release 4

Type: Bug
Reporter: Earthcomputer Assignee: [Mojang] Nathan Adams
Resolution: Fixed Votes: 4
Labels: /execute, /function, command
Environment:
  • OS: Windows 10
  • Java: 1.8.0_112

Issue Links:
Relates
relates to MC-117562 Executing a function as an entity doe... Resolved
Confirmation Status: Unconfirmed

 Description   

Description

When executing the following command:

/function test:test

and the definition of test:test is as follows:

execute @s ~ ~ ~ function test:test

you get this error message in the chat and server log:

An unknown error occurred while attempting to perform this command

Note: this also gets around the maxCommandChainLength limit (untested, but assumed true based on the code analysis below).

Edit: as of 1.12-pre3, the execute command must be the last command in the function

Why this is a problem

Though the example above may be solvable by just replacing the execute command with a function command, it turns out that the execute command is the only way to create real conditionals, terminating loops and recursive functions. If I have a recursive function equivalent to a for loop from 1-1000, for example, it takes up a lot of space on the stack (see code analysis below). It may even be possible for more complex functions to run out of stack space, which kind of defeats the point of the maxCommandChainLength gamerule.

Code analysis

The bug is probably caused by a StackOverflowError. The current code used to execute functions is quite clever: it treats function commands inside functions differently to other commands such to avoid stack overflows from occurring. This is why we need it inside an execute command in the function definition, so the game doesn't recognize it as a function command.

What calls what:

  • CommandFunction.execute(MinecraftServer, ICommandSender, String[]) (line 46)
  • FunctionManager.runFunction(Function, ICommandSender) (line 114)
  • FunctionManager$LineOfCode.execute(ArrayDeque<LineOfCode>, int) (line 176)
  • Function$CommandInsn.execute(FunctionManager, ICommandSender, ArrayDeque<LineOfCode>, int) (line 60)
  • AbstractCommandManager.execute(ICommandSender, String) (line 62)
  • AbstractCommandManager.tryExecute(ICommandSender, String[], ICommand, String) (line 92)
  • CommandExecute.execute(MinecraftServer, ICommandSender, String[]) (line 118)
  • CommandFunction.execute(MinecraftServer, ICommandSender, String[]) (line 46)
  • ...

The current implementation of FunctionManager.runFunction looks like this:

public int runFunction(Function func, ICommandSender sender)
{
    int maxChainLength = getMaxCommandChainLength();
    // Attempted fix for this bug (I think)
    // This should also be a >= 1 (if I'm not mistaken)
    if (this.linesLeftToExecute.size() > 1)
    {
        if (this.linesLeftToExecute.size() < maxChainLength)
        {
            this.linesLeftToExecute.addFirst(new LineOfCode(this, sender, new FunctionInsn(func)));
        }
        return 0;
    }
    
    try {
        int linesExecuted = 0;
        // ... read function to add all lines ...
        while (!this.linesLeftToExecute.isEmpty())
        {
            // Since the line of code is removed *before* it is executed,
            // linesLeftToExecute could be empty before we reach the lines above again
            this.linesLeftToExecute.removeFirst().execute(this.linesLeftToExecute, maxChainLength);
            if (++linesExecuted >= maxChainLength)
            {
                return linesExecuted;
            }
        }
        return linesExecuted;
    } finally {
        this.linesLeftToExecute.clear();
    }
}

Alternative solution

(doesn't fix bug, but add a way round it)
Add proper support for conditionals in functions



 Comments   
Comment by Earthcomputer [ 13/May/17 ]

The code above as it is slightly changes the functionality of the execute command. I'll edit the post a bit later when I have more time.
Edit: fixed

Comment by [Mod] Marcono1234 [ 13/May/17 ]

Couldn't you do the following to prevent StackOverflows?

Add a executingCommand flag for net.minecraft.command.CommandHandler which is set to true in the method net.minecraft.command.CommandHandler.executeCommand(ICommandSender, String) before a command is executed and to false once the command was executed. Then add a ArrayDeque containing commands with their sender. When a command should be run while another command is currently running add that comment to the array. And after the currently running command finishes (but before the executingCommand flag is set to false) process all commands in the array.

Comment by Earthcomputer [ 13/May/17 ]

That may indeed be a better solution.
What may be even better is to have the ICommandSender execute the command itself. The default implementation of ICommandSender.execute(String[] command) just delegates to CommandHandler.executeCommand (because we have default methods in Java 8), but FunctionCommandSender could override this to add the command to the deque instead if necessary. That way we're still keeping track of the maxCommandChainLength.
What also makes yours and this new solution better than my original proposed solution is that it would work for any command capable of running other commands, including /function, /execute and any others which may be added in the future. There will be no special cases in the function-executing code anymore.

Comment by Earthcomputer [ 15/May/17 ]

On second thoughts, a cancellable ICommandSender.onCommandExecuted might be better

Comment by Earthcomputer [ 17/May/17 ]

It says it's fixed, though it still happens on 1.12-pre3

Comment by Earthcomputer [ 17/May/17 ]

I have replaced the old suggested fix in the original bug report to a stack-trace-type-deal.

Comment by Earthcomputer [ 17/May/17 ]

Added more code analysis

Comment by Earthcomputer [ 17/May/17 ]

I've done some more research into this. It seems that the bug was indeed partially fixed for 1.12-pre3, but not for all cases.
The bug now occurs if and only if the execute command is the last command in the function. I think I have identified the cause of this in the code analysis above.
This does mean however that there is a temporary workaround for map-makers:

# ... lots of commands ...
# Execute command that *must* be at the end of the function
execute ... function ...
# Append a real command but with invalid syntax
scoreboard

This works because the execute command is no longer the last command in the function. It does however mean that we reach the maxCommandChainLength quicker with a dummy command we're not even using though...

Generated at Mon Oct 15 16:11:25 CDT 2018 using Jira 7.11.2#711002-sha1:fdc329dee91471a641faabfe39b5ff8c0a5b3f66.