Skip to content

Commit

Permalink
fix: NetworkIdentity.OnStartLocalPlayer catches exceptions now too. f…
Browse files Browse the repository at this point in the history
…ixes a potential bug where an exception in PlayerInventory.OnStartLocalPlayer would cause PlayerEquipment.OnStartLocalPlayer to not be called
  • Loading branch information
miwarnec committed Feb 28, 2020
1 parent 4926bb8 commit 5ed5f84
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
14 changes: 13 additions & 1 deletion Assets/Mirror/Runtime/NetworkIdentity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,19 @@ internal void OnStartLocalPlayer()

foreach (NetworkBehaviour comp in NetworkBehaviours)
{
comp.OnStartLocalPlayer();
// an exception in OnStartLocalPlayer should be caught, so that
// one component's exception doesn't stop all other components
// from being initialized
// => this is what Unity does for Start() etc. too.
// one exception doesn't stop all the other Start() calls!
try
{
comp.OnStartLocalPlayer();
}
catch (Exception e)
{
Debug.LogError("Exception in OnStartClient:" + e.Message + " " + e.StackTrace);
}
}
}

Expand Down
22 changes: 21 additions & 1 deletion Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ class StopAuthorityCalledNetworkBehaviour : NetworkBehaviour
public override void OnStopAuthority() { ++called; }
}

class StartLocalPlayerExceptionNetworkBehaviour : NetworkBehaviour
{
public int called;
public override void OnStartLocalPlayer()
{
++called;
throw new Exception("some exception");
}
}

class StartLocalPlayerCalledNetworkBehaviour : NetworkBehaviour
{
public int called;
Expand Down Expand Up @@ -855,16 +865,26 @@ public void OnStartLocalPlayer()
// create a networkidentity with our test component
GameObject gameObject = new GameObject();
NetworkIdentity identity = gameObject.AddComponent<NetworkIdentity>();
StartLocalPlayerExceptionNetworkBehaviour compEx = gameObject.AddComponent<StartLocalPlayerExceptionNetworkBehaviour>();
StartLocalPlayerCalledNetworkBehaviour comp = gameObject.AddComponent<StartLocalPlayerCalledNetworkBehaviour>();

// call OnStartLocalPlayer on identity should call it in comp
// make sure our test values are set to 0
Assert.That(compEx.called, Is.EqualTo(0));
Assert.That(comp.called, Is.EqualTo(0));

// call OnStartLocalPlayer in identity
// one component will throw an exception, but that shouldn't stop
// OnStartLocalPlayer from being called in the second one
LogAssert.ignoreFailingMessages = true; // exception will log an error
identity.OnStartLocalPlayer();
LogAssert.ignoreFailingMessages = false;
Assert.That(compEx.called, Is.EqualTo(1));
Assert.That(comp.called, Is.EqualTo(1));

// we have checks to make sure that it's only called once.
// let's see if they work.
identity.OnStartLocalPlayer();
Assert.That(compEx.called, Is.EqualTo(1)); // same as before?
Assert.That(comp.called, Is.EqualTo(1)); // same as before?

// clean up
Expand Down

0 comments on commit 5ed5f84

Please sign in to comment.