Fix PlayerLifecycleHelper.IsOwningWorker (#1027)

Author: Jamie Brynes

CHANGELOG.md

@@ -15,6 +15,15 @@
     - Use the `buildTargetFilter` command line argument to pass in a comma delimited list of build targets to filter for. For example, `+buildTargetFilter win,macos`.
 - Added two new GDK packages: `io.improbable.worker.sdk` and `io.improbable.worker.sdk.mobile` which contain the underlying Worker SDK packages for Windows/MacOS/Linux and Android/iOS respectively.
 
+
+### Changed
+
+- `PlayerLifecycleHelper.IsOwningWorker` will now return false instead of throwing an exception if the entity is not in your worker's view.
+
+### Fixed
+
+- Fixed a bug where `PlayerLifecycleHelper.IsOwningWorker` would throw an exception if the entity was in your worker's view.
+
 ### Internal
 
 - Stopped throwing a `Test Exception` in playground.

test-project/Assets/EditmodeTests/Improbable.Gdk.EditModeTests.asmdef

@@ -5,7 +5,8 @@
         "Improbable.Gdk.Generated",
         "Improbable.Gdk.Core",
         "Unity.Entities",
-        "Unity.Entities.Hybrid"
+        "Unity.Entities.Hybrid",
+        "Improbable.Gdk.PlayerLifecycle"
     ],
     "optionalUnityReferences": [
         "TestAssemblies"
@@ -18,5 +19,6 @@
     "overrideReferences": false,
     "precompiledReferences": [],
     "autoReferenced": true,
-    "defineConstraints": []
-}
+    "defineConstraints": [],
+    "versionDefines": []
+}

test-project/Assets/EditmodeTests/PlayerLifecycle.meta

@@ -0,0 +1,75 @@
+using System;
+using Improbable.Gdk.Core;
+using Improbable.Gdk.PlayerLifecycle;
+using NUnit.Framework;
+using UnityEngine;
+
+namespace Improbable.Gdk.EditmodeTests.PlayerLifecycle
+{
+    [TestFixture]
+    public class PlayerLifecycleHelperTests
+    {
+        private SpatialOSReceiveSystem receiveSystem;
+        private MockConnectionHandler connectionHandler;
+        private WorkerInWorld worker;
+
+        [SetUp]
+        public void Setup()
+        {
+            var builder = new MockConnectionHandlerBuilder();
+            connectionHandler = builder.ConnectionHandler;
+
+            worker = WorkerInWorld.CreateWorkerInWorldAsync(builder, "TestWorkerType", new LoggingDispatcher(),
+                Vector3.zero).Result;
+
+            receiveSystem = worker.World.GetExistingSystem<SpatialOSReceiveSystem>();
+        }
+
+        [Test]
+        public void IsOwningWorker_should_return_false_if_entity_does_not_exist()
+        {
+            Assert.IsFalse(PlayerLifecycleHelper.IsOwningWorker(new EntityId(10), worker.World));
+        }
+
+        [Test]
+        public void IsOwningWorker_should_return_false_if_entity_doesnt_have_OwningWorker_component()
+        {
+            connectionHandler.CreateEntity(1, new EntityTemplate());
+            receiveSystem.Update();
+
+            Assert.IsFalse(PlayerLifecycleHelper.IsOwningWorker(new EntityId(1), worker.World));
+        }
+
+        [Test]
+        public void IsOwningWorker_should_return_false_if_entity_isnt_owned_by_this_worker()
+        {
+            connectionHandler.CreateEntity(1, GetOwnedEntity("other-worker"));
+            receiveSystem.Update();
+
+            Assert.IsFalse(PlayerLifecycleHelper.IsOwningWorker(new EntityId(1), worker.World));
+        }
+
+        [Test]
+        public void IsOwningWorker_should_return_true_if_OwningWorker_component_has_my_worker_id()
+        {
+            connectionHandler.CreateEntity(1, GetOwnedEntity(worker.WorkerId));
+            receiveSystem.Update();
+
+            Assert.IsTrue(PlayerLifecycleHelper.IsOwningWorker(new EntityId(1), worker.World));
+        }
+
+        [TearDown]
+        public void TearDown()
+        {
+            worker.Dispose();
+        }
+
+        private EntityTemplate GetOwnedEntity(string workerId)
+        {
+            var template = new EntityTemplate();
+            template.AddComponent(new Position.Snapshot(), "worker");
+            template.AddComponent(new OwningWorker.Snapshot(workerId), "worker");
+            return template;
+        }
+    }
+}

test-project/Assets/EditmodeTests/PlayerLifecycle/PlayerLifecycleHelperTests.cs

@@ -7,6 +7,7 @@
     "io.improbable.gdk.testutils": "file:../../workers/unity/Packages/io.improbable.gdk.testutils",
     "io.improbable.gdk.tools": "file:../../workers/unity/Packages/io.improbable.gdk.tools",
     "io.improbable.worker.sdk": "file:../../workers/unity/Packages/io.improbable.worker.sdk",
+    "io.improbable.gdk.playerlifecycle": "file:../../workers/unity/Packages/io.improbable.gdk.playerlifecycle",
     "com.unity.modules.physics": "1.0.0"
   },
   "registry": "https://staging-packages.unity.com"

test-project/Assets/EditmodeTests/PlayerLifecycle/PlayerLifecycleHelperTests.cs.meta

@@ -33,18 +33,16 @@ public static bool IsOwningWorker(EntityId entityId, World workerWorld)
                 throw new InvalidOperationException("Provided World does not have an associated worker");
             }
 
-            if (entitySystem.GetEntitiesInView().Contains(entityId))
+            if (!entitySystem.GetEntitiesInView().Contains(entityId))
             {
-                throw new InvalidOperationException(
-                    $"Entity with SpatialOS Entity ID {entityId.Id} is not in this worker's view");
+                return false;
             }
 
             if (!updateSystem.HasComponent(OwningWorker.ComponentId, entityId))
             {
                 return false;
             }
 
-
             var ownerId = updateSystem.GetComponent<OwningWorker.Snapshot>(entityId).WorkerId;
             return worker.WorkerId == ownerId;
         }