mojira.dev
MC-117562

Executing a function as an entity doesn't use new sender

Add a function with a command such as setblock ~ ~2 ~ stone or say @s.

In the world place a command block and run the function as a player (or other entity):
/execute @p ~ ~ ~ function foo:bar

The expected behaviour is that a stone block is placed above the nearest player, however, it is placed at the command block instead. As well, using @s in the function does nothing, as the command block is the sender.

Affects similar commands in other contexts as well.

Linked issues

Attachments

Comments 14

Attached some images shown in my (duplicate) report: https://bugs.mojang.com/browse/MC-117588

In my report, I tried to get a cow to say hi using the function Tyruswoo:Hi
This worked in 1.12pre1 and 1.12pre2, but in 1.12pre3 it was broken, making me (the executor) say hi. My intent was for the cow (the target of the execute command) to say hi.

The problem is, in 1.12 they added a new type of ICommandSender, let's call it DelegatingCommandSender, which, as the name suggests, delegates to another command sender. It's used by the execute command and functions.
Now, the execute command overrides the ICommandSender.getEntity() method to return the entity matched by the selector, so this mostly works fine. However, in the case that another DelegatingCommandSender delegates to this DelegatingCommandSender, which in turn delegates to the executor of the execute command (in your example the command block), there is a problem.
There is currently an oversight in DelegatingCommandSender. If it's told to delegate to another DelegatingCommandSender, it thinks "oh look! another delegating command sender. That's just delegating somewhere else, there's no point going through it, so I'm just going to bypass it and delegate to its child delegate instead". It does not take into account that the execute command and functions have subclassed DelegatingCommandSender.
So, in conclusion:

  1. The command block runs the execute command

  2. The execute command creates a DelegatingCommandSender which delegates to the command block

  3. The execute command also runs the function command

  4. The function code creates a DelegatingCommandSender, which bypasses the other delegating command sender all the way back to the command block.

  5. The function executes say @s

  6. The say command needs to know who '@s' is, so it asks the command sender

  7. The DelegatingCommandSender says "I don't know, but my delegate will! I'll hand you over to him, Mr. Command Block over there"

  8. The command block says "'@s'?! I don't know what you're talking about! I'm not an entity!"

  9. The say command does nothing because no entity was matched

That sounds really complex! It seems pretty simple to just let @s refer to the target of /execute, but I guess there are a lot of things going on in the background! Really hope this can be fixed!

By the way, this bypassing of delegation is important to prevent StackOverflowError s from occurring while executing recursive functions. So if you reverse it you break something else, so it's a kind of catch 22.
The solution that springs to my mind is to do away with delegation and, it might be possible to store all the ICommandSender s on a stack instead.

4 more comments

Just want to throw this in here:

public interface ICommandSender
{
    // ...
    default boolean equals(Object other)
    {
        // ... check to see if other is a ICommandSender ...
        ICommandSender sender = (ICommandSender) other;
        return getEntity().equals(sender.getEntity())
            && getPosition().equals(sender.getPosition())
            // ...
            && getPositionVector().equals(sender.getPositionVector());
    }
}

public class DelegatingCommandSender implements ICommandSender
{
    private ICommandSender delegate;

    public DelegatingCommandSender(ICommandSender delegate)
    {
        this.delegate = delegate;
        this.delegate = getDeepestDelegate();
    }
    
    private ICommandSender getDeepestDelegate()
    {
        ICommandSender tmp = this.delegate;
        ICommandSender newDelegate = this.delegate;
        while (tmp instanceof DelegatingCommandSender)
        {
            tmp = ((DelegatingCommandSender) tmp).delegate;
            // Now we add this check here
            if (this.delegate.equals(tmp))
            {
                newDelegate = tmp;
            }
        }
        return newDelegate;
    }
    // ...
}

As I was typing that both bugs got fixed haha

Great! Yeah, if both are fixed, that's even better!

Just checked that it works with the Cow Hi test, and uploaded a photo!

Misterx7772

Nathan Adams

Confirmed

/execute, /function, entity

Minecraft 1.12 Pre-Release 3

Minecraft 1.12 Pre-Release 4

Retrieved