Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distributed Authority NetworkTransform errors after claiming transferrable ownership #2897

Open
Yoraiz0r opened this issue Apr 25, 2024 · 4 comments
Assignees
Labels
priority:medium stat:imported Issue is tracked internally at Unity type:bug Bug Report

Comments

@Yoraiz0r
Copy link

Yoraiz0r commented Apr 25, 2024

Description

Using NGO 2.0.0-exp2 and Distributed Authority, claiming authority of a moving Transferrable object that relies on NetworkTransform (such as with NetworkRigidbody on a simple sphere), will cause an error.

"Authority receiving transform update from Client-1!"
The faulting method is Unity.Netcode.Components.NetworkTransform:TransformStateUpdate which checks if you can commit to a transform - meaning it is assumed to be yours. But if you take immediate ownership while there are still transform packets coming your way, that error will trigger.

The way ownership has been claimed to cause this, is with the following code:

	if (!other.HasAuthority)
		other.ChangeOwnership(me.OwnerClientId);

(there are other predicates prior to this eliminating non-transferrable, non-specific objects)

Calling this code on any moving Transferrable Network Object that uses NetworkTransform such as NetworkRigidbody will trigger the issue, though unreliably, as you have to time it to the sending frequency of the NetworkTransform update.

Reproduce Steps

  1. Create a new project with NGO2.0.0-exp2
  2. Create a new scene with a moving scene object (such as a falling sphere), give it NetworkObject, set it as Distributable and Transferrable, and give it a NetworkRigidbody.
  3. Add a NetworkManager and whatever you need for a non-owning client to connect
  4. Add a way to trigger the above script such that the client could take authority of the sphere (such as with a button press)
  5. Enter playmode for two clients (such as with Multiplayer Play Mode)
  6. Let the non-hosting client call ChangeOwnership on the object, and witness the error occurring. If it did not occur, you may need to retry this a few times. (timing to the networktransform packets)

Actual Outcome

NetworkTransform logs an error immediately after claiming ownership

Expected Outcome

No error occurs when claiming ownership of a transferrable object.

Screenshots

I've included a short video of my own test here.

This test is using NGO 2.0.0-exp2 and Kinematic Character Controller's OnDiscreteCollisionDetected method to detect touching and shifting of authority.

Environment

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

Additional Context

I've witnessed that NGO 2.0.0-exp2's changelog mentions a recommendation for using RigidbodyContactEventManager and mentioning this to be useful for distributed authority, though I am failing to see its primary use case. I'd love if there would be a lite sample that explains the proper usage of this.

In attempt to mitigate the problem above, I tried to figure out how to use the manager and stumbled upon these in trial and error:

  1. The manager has nothing to do with networking, it is simply an event processor
  2. For an event to be processed, both participants of the contact event must be registered. As a rigidbody is used for InstanceID keying, it is impossible to register in-place colliders for the purpose of using this manager. (Without giving them a rigidbody, which you can still set to kinematic, so its fine)
  3. You have to (and can only) include 1 event handler per Rigidbody. This makes it difficult to make components that can stack behaviors to occur when using the manager. (Though it is possible to create a single register call that other components can register to, so that's fine)

It is still unclear to me when should I register a class to this event manager

  • OnEnable & OnDisable seems too early, may not have an instance of the event manager assigned yet if the event handler is a scene object
  • Start & OnDestroy seems fine, but feels wrong, may get event calls when not actually logged into a server, which would require redundant checks

For now I've opted to use OnNetworkSpawn & OnNetworkDespawn, I hope that's the proper use, since I could not find a single piece of documentation on how to use the contact events manager.

I've made these classes:

public class EmptyContactEventHandler : NetworkBehaviour, IContactEventHandler
{
	[SerializeField] private Rigidbody _rigidbody;
	public Rigidbody GetRigidbody() => _rigidbody;

	public override void OnNetworkSpawn() => RigidbodyContactEventManager.Instance.RegisterHandler(this);
	public override void OnNetworkDespawn() => RigidbodyContactEventManager.Instance.RegisterHandler(this, false);

	public void ContactEvent(ulong eventId, Vector3 averagedCollisionNormal, Rigidbody collidingBody,
		Vector3 contactPoint,
		bool hasCollisionStay = false, Vector3 averagedCollisionStayNormal = default)
	{
	}
}
public class TransferOwnershipToLocalClientContactResolver : NetworkBehaviour, IContactEventHandler
{
	[SerializeField] private Rigidbody _rigidbody;
	public Rigidbody GetRigidbody() => _rigidbody;

	public override void OnNetworkSpawn() => RigidbodyContactEventManager.Instance.RegisterHandler(this);
	public override void OnNetworkDespawn() => RigidbodyContactEventManager.Instance.RegisterHandler(this, false);


	public void ContactEvent(ulong eventId, Vector3 averagedCollisionNormal, Rigidbody collidingBody, Vector3 contactPoint,
		bool hasCollisionStay = false, Vector3 averagedCollisionStayNormal = default)
	{
		if (HasAuthority) //local client already owns this
			return;

		if (hasCollisionStay) //already have a first-contact event, if I understand this correctly
			return;
		
		if (!collidingBody.TryGetComponent(out NetworkObject obj)) //touched by something that has no owner, ignore
			return;

		if (OwnerClientId != obj.OwnerClientId)
		{
			Debug.Log($"Changing ownership! {name}, {OwnerClientId}, {obj.OwnerClientId}");
			NetworkObject.ChangeOwnership(obj.OwnerClientId);
		}
	}
}

However, the issue persisted, which makes sense - it has nothing to do with the way in which contacts are polled, just the order of events that happen.

  • Remote client owns a networked transform
  • Remote client sends a networked transform packet
  • Local client takes ownership of networked transform
  • Local client receives networked transform packet, which under normal Client Server operations, should not happen, so it errors.

I've also read the available documentation on ownership race conditions, but it was not helpful.

  • I am using the recommended Transferable flag
  • Locking ownership would not prevent the transform packet from arriving late

Would love to know how to proceed with this issue for my own project, as the lag of not setting ownership immediately on contact makes the player experience poor. (Sphere acting as a kinematic body and not pushable if there is no ownership, even a 0.3 second delay is felt awfully)

@Yoraiz0r Yoraiz0r added stat:awaiting triage Status - Awaiting triage from the Netcode team. type:bug Bug Report labels Apr 25, 2024
@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Apr 26, 2024

@Yoraiz0r
Thank you for the detailed feedback and replication steps.
I am going to investigate the first scenario you described and get back to you on that.

Regarding this:

Would love to know how to proceed with this issue for my own project, as the lag of not setting ownership immediately on contact makes the player experience poor. (Sphere acting as a kinematic body and not pushable if there is no ownership, even a 0.3 second delay is felt awfully)

If the NetworkObject is set to transferrable, then you can :

  • Use triggers that are slightly larger than each body's collider
  • Upon a player entering the trigger, on that local client, just transfer ownership
  • You can do the same with just colliders, but the trigger approach assures it is non-kinematic prior to the colliders...colliding.

@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Apr 26, 2024

The error message you are talking about does indeed need to be removed, but I believe was a reminder (for myself) to assure it did not impact the new owner. As you can see in the source it just logs the error and continues processing:

        internal void TransformStateUpdate(ref NetworkTransformState networkTransformState, ulong senderId)
        {
            if (CanCommitToTransform)
            {
                Debug.LogError($"Authority receiving transform update from Client-{senderId}!");
            }
            // Store the previous/old state
            m_OldState = m_LocalAuthoritativeNetworkState;

            // Assign the new incoming state
            m_LocalAuthoritativeNetworkState = networkTransformState;

            // Apply the state update
            OnNetworkStateChanged(m_OldState, m_LocalAuthoritativeNetworkState);
        }

I will make sure that message no longer occurs in the next exp release and I REALLY appreciate your feedback on all of this! 👍

(still reviewing your questions about RigidbodyContactEventManager, although that is a very early feature that still needs some work and we don't have any documentation for that jus yet...)

@Yoraiz0r
Copy link
Author

@NoelStephensUnity
Thank you for the frequent responses to the issues!

Is it correct to assume that if I am in distributed authority mode, I can just return early before the LogError? Since the client already claimed ownership I don't see a need to accept or process the late message, is that correct?

As a related followup, I recognize that Distributed Authority is ill suited for physics based & competitive games since you don't actually get any ground truth on what happened in the simulation, but I'm bumping into quite a connundrum with some innocent "worst case scenario"...

Using the original "when player touches a sphere, they start owning that sphere", it becomes impossible for 2 players to try and push the same object at once, like this example.

The issue here happens with the leftside player touching the sphere, claiming ownership of it immediately, repeatedly, such that the rightside player cannot push it.

If I were to use an ownership lock until there's no other player in the general area then they still won't be able to push the sphere simultaneously.

I've tried setting the Network Rigidbody's Auto Update Kinematic State off which leaves it non-kinematic throughout, but I'm noticing other behavioral problems showing as a result, though the overall behavior is closer to expected!

Would love to know a recommendation for this case, because it seems this was tested for the unity 6 roadmap video here.

Thanks again!

@fluong6 fluong6 added stat:import priority:medium and removed stat:awaiting triage Status - Awaiting triage from the Netcode team. labels Apr 29, 2024
@fluong6 fluong6 added stat:imported Issue is tracked internally at Unity and removed stat:import labels Apr 29, 2024
@NoelStephensUnity
Copy link
Collaborator

Is it correct to assume that if I am in distributed authority mode, I can just return early before the LogError? Since the client already claimed ownership I don't see a need to accept or process the late message, is that correct?

Yeah, basically you grabbed ownership while packets from the authority were in flight.
I am still noodling on a way to make this an optional thing to be applied (i.e. leave it up to developers), but yeah... as long as the message isn't something like a teleport you should be fine to drop the message.

As a related followup, I recognize that Distributed Authority is ill suited for physics based & competitive games since you don't actually get any ground truth on what happened in the simulation, but I'm bumping into quite a connundrum with some innocent "worst case scenario"...

Using the original "when player touches a sphere, they start owning that sphere", it becomes impossible for 2 players to try and push the same object at once, like this example.

First... I want to say (again) thank you for playing around with the experimental package! This kind of feedback is great. 💯

Regarding the issue with "fighting" between two colliding bodies... yeah that is always a tough problem (given the scenario) to solve for without adding some "rules" to these edge cases.

You can always "lock" a NetworkObject using NetworkObject.SetOwnershipLock which just prevents one of the two bodies from grabbing ownership. Of course, the context of the game play mechanic depends on how/when you would want to do this.

Upon colliding with and/or triggering a collider you can do a quick check to see if the current owner is within a specific range of the NetworkObject and if so don't grab ownership.

Regarding two players pushing the same object at the same time... that is a trickier problem to solve since you want both sides to impact the over-all velocity and to have a simulation running locally on both clients (or more than 2 even). This is something I haven't yet gotten to...and in reality might not be in the pre-release version since there are plenty of other areas that still need some polish (you have found several of them)...

Would love to know a recommendation for this case, because it seems this was tested for the unity 6 roadmap video here.

The video you are seeing is one of the samples (soon to be released) that includes some helper components:

  • BaseObjectMotionHandler: A base NetworkBehaviour class that handles distributed authority motion and collision events
  • PhysicsObjectMotion: Derived from BaseObjectMotionHandler, this handles physics related synchronization (i.e. it auto-synchronizes force and velocities for the Rigidbody) and uses RigidbodyContactEventManager.

I can say that those spheres moving around transfer ownership using trigger colliders that are slightly bigger than the actual collider (i.e. it assures the object becomes non-kinematic before a collision occurs).

If I were to use an ownership lock until there's no other player in the general area then they still won't be able to push the sphere simultaneously.

With the previous info about using triggers... you could look into a similar type of scenario where the "first to make contact" maintains "ownership" (i.e. locked) until <insert some condition here like the object exceeds (n) distance>. Then you could use a trigger to apply an "artificial" velocity via RPC (SendTo.Authority) for any other client that gets within the trigger distance...

But.. that would be a temporary approach until I get the time to focus on the more complex physics scenarios (i.e. more than one client applying a force/velocity to an object at the same time).

There are a bunch of things I would like to do (i.e. indirect forces applied... where you could take ownership of a sphere...give it velocity by pushing it... and then have that sphere collide with other spheres not owned by you and still apply an impact force)... but first things first... 😄

Still need to get the foundational parts cleaned up a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:medium stat:imported Issue is tracked internally at Unity type:bug Bug Report
Projects
None yet
Development

No branches or pull requests

3 participants