Granting the Distributed Authority Host ownership of a gameobject from a remote client erronously triggers a warning

[Yoraiz0r]

Description

Using NGO 2.0.0-exp2 with Distributed Authority,
Assuming Alice hosts with Distributed Authority mode,
And Bob joins,
And there is a NetworkObject owned by Bob with the Transferrable flag, without Requires Permission,
And Bob calls ChangeOwnership(Alice)
Bob will get a warning Unnecessary ownership changed message for {NetworkObjectId}

Reproduce Steps

1. Create a new project and import the NGO 2.0.0-exp2 package
2. Create a new scene with a NetworkManager in Distributed Authority mode, and buttons to host, and join said host
3. Create a NetworkBehaviour that allows to send ownership of the object holding it to another client (Such as Alice to Bob, Bob to Alice) by calling NetworkObject.ChangeOwnership(otherClientId);
4. Create a NetworkObject with the Transferrable flag, but without Request Required flag, and add the NetworkBehaviour
5. Enter playmode, have a host and a client joined to it
6. Make the host to transfer the object to the client, and witness everything is going well
7. Make the client transfer the object to the host, and witness that the client receives a warning of Unnecessary ownership changed message for {NetworkObjectId}.

Actual Outcome

Client gets a warning after transferring ownership of its object to the host

Expected Outcome

Client gets no warning after transferring ownership of its object to the host

Environment

• OS: Windows 10
• Unity Version: 6000.0.0b13
• Netcode Version: 2.0.0-exp2

Additional Context

I've locally pulled the package and checked the cause,
This is because the Distributed Authority Host (DAHost), in Unity.Netcode.ChangeOwnershipMessage.Handle, is set to relay messages to other clients, with checks to skip under certain conditions, but this specific case is excluded.
I've cleaned up the massive if check in it to the following:

                        // If ownership is changing and this is not an ownership request approval then ignore the OnwerClientId
                        var case1 = OwnershipIsChanging && !RequestApproved && OwnerClientId == clientId;

                        // If it is just updating flags then ignore sending to the owner
                        var case2 = OwnershipFlagsUpdate && clientId == OwnerClientId;

                        // If it is a request or approving request, then ignore the RequestClientId
                        var case3 = (RequestOwnership || RequestApproved) && clientId == RequestClientId;

Case 1 assumes that we should ignore the Owner, instead of the Sender.
This is erroneous, for one of two reasons:

• Either because it assumes that you can only change ownership towards yourself.
• Or because the ChangeOwnership method should not be exposed, and a new method ClaimOwnership that forces it towards the local client, should be exposed.

For the time being, I've added a 4th case like so:

                        // If ownership is changing and it is a transfer then ignore the sender
                        var case4 = OwnershipIsChanging && !RequestApproved && context.SenderId == clientId;

I'm not sure if this is entirely correct, but it feels correct for and resolves the problem for the time being.

[NoelStephensUnity]

@Yoraiz0r
This we were aware of for the DAHost.
The DAHost mode is something that is more for development purposes than it is for a live service connection.
(which will be available to use soon)

However, the next exp release will have this issue removed.
(Thank you for the detailed explanation) 👍