FishNet icon indicating copy to clipboard operation
FishNet copied to clipboard

Update NetworkCollider.cs

Open jimdroberts opened this issue 1 year ago • 3 comments

Add Query Trigger Interaction support for overlaps. Add PhysicsScene support for overlaps. Add OnEnterOnce which is guaranteed to be called at least once when an object enters this collider. Add _currentlyEntered, a new hashset that contains the currently entered hits. This allows us to invoke OnEnterOnce multiple times until a collision is considered successful(true). Renamed previous to previouslyHit for clarity.

Issue: NetworkCollider.OnEnter may never reach the Debug.Log call on the Client if base.PredictionManager.IsReconciling is true on the first collision of the NetworkCollider as all hits are added to _colliderDataHistory. This has been observed on a CSP movement controller entering a NetworkCollider immediately after spawning into a scene.

private void NetworkTrigger_OnTriggerEnter(Collider other)
{
	if (base.PredictionManager.IsReconciling)
	{
		return;
	}
	Debug.Log($"{other.gameObject.name} Entered {gameObject.name}");
}

Fix: NetworkCollider.OnEnterOnce will attempt multiple invocations while a collider remains inside of the NetworkCollider until true. This ensures that the function will be invoked at least once while a collider remains inside of the NetworkCollider.

private bool NetworkTrigger_OnTriggerEnterOnce(Collider other)
{
	if (base.PredictionManager.IsReconciling)
	{
		return false;
	}
	Debug.Log($"{other.gameObject.name} Entered {gameObject.name}");
	return true;
}

jimdroberts avatar May 08 '24 06:05 jimdroberts

OnEnter was left in for testing purposes but should be marked obsolete upon further testing.

QueryTriggerInteraction.Collide allows Trigger<->Trigger hits and Trigger<->Collider hits if the current collider is a trigger. This coincides more with the Unity physics engine and removes the need for NetworkCollider.IsTrigger as it is determined instead by the current colliders trigger settings. As a result the NetworkTrigger and NetworkCollider types are no longer needed.

Side Note: I may have a bug in my CSP movement controller that is causing reconcile to evaluate true on the client but these changes fix this issue regardless.

jimdroberts avatar May 08 '24 07:05 jimdroberts

This needs to be reviewed more thoroughly given the changes it presents. I'll likely get in touch with you on this PR.

FirstGearGames avatar May 11 '24 18:05 FirstGearGames

I added in the layers requests, which I did not see as part of the PR. I just wanted to let you know I've not forgotten.

FirstGearGames avatar Jun 04 '24 19:06 FirstGearGames

Need to run tests with you before this can be approved.

FirstGearGames avatar Jul 12 '24 16:07 FirstGearGames

We've not really made any progress on this so I'm going to close it out. I've not had any reports for a long while of the most recent being broken either.

I do think it would be cool to a 'fire once' option though; it would need to be implemented a little differently but doable.

FirstGearGames avatar Sep 13 '24 17:09 FirstGearGames