Requireable constraint tweaks & bugfixing

CHANGELOG.md

@@ -20,6 +20,8 @@
 - Fixed a bug where dotnet output from the code generator would cause exceptions to be thrown. [#1294](https://github.com/spatialos/gdk-for-unity/pull/1294)
 - Fixed a bug where the Mobile Launcher window wouldn't find Android devices that contained hyphens in their product name. [#1288](https://github.com/spatialos/gdk-for-unity/issues/1288) [#1296](https://github.com/spatialos/gdk-for-unity/pull/1296)
 - Fixed a bug where component events were not dropped properly when the entity-component pair was removed from the View. [#1298])(https://github.com/spatialos/gdk-for-unity/pull/1298)
+- Fixed a bug where Reader/Writer/CommandSender/CommandReceiver fields would not have their state set to invalid when the underlying constraints were not met. [#1297](https://github.com/spatialos/gdk-for-unity/pull/1297)
+    - This bug would manifest itself in situations like a `Reader` reference attempting to read data that does not exist in your worker's view anymore.
 
 ### Removed
 

test-project/Assets/EditmodeTests/Subscriptions/RequireablesDisableTests.cs

@@ -0,0 +1,142 @@
+using Improbable.Gdk.Core;
+using Improbable.Gdk.Subscriptions;
+using Improbable.Gdk.Test;
+using Improbable.Gdk.TestBases;
+using Improbable.Worker.CInterop;
+using NUnit.Framework;
+using UnityEngine;
+
+namespace Improbable.Gdk.EditmodeTests.Subscriptions
+{
+    public class RequireablesDisableTests : SubscriptionsTestBase
+    {
+        private const long EntityId = 100;
+        private GameObject createdGameObject;
+
+        [SetUp]
+        public override void Setup()
+        {
+            base.Setup();
+            var template = new EntityTemplate();
+            template.AddComponent(new Position.Snapshot(), "worker");
+            template.AddComponent(new TestCommands.Snapshot(), "worker");
+            ConnectionHandler.CreateEntity(EntityId, template);
+            Update();
+        }
+
+        [Test]
+        public void Reader_is_disabled_if_component_removed()
+        {
+            createdGameObject = CreateAndLinkGameObjectWithComponent<PositionReaderBehaviour>(EntityId);
+            var reader = createdGameObject.GetComponent<PositionReaderBehaviour>().Reader;
+
+            ConnectionHandler.RemoveComponent(EntityId, Position.ComponentId);
+            Update();
+
+            Assert.IsNull(createdGameObject.GetComponent<PositionReaderBehaviour>().Reader);
+            Assert.IsFalse(reader.IsValid);
+        }
+
+        [Test]
+        public void Writer_is_disabled_if_loses_auth()
+        {
+            ConnectionHandler.ChangeAuthority(EntityId, Position.ComponentId, Authority.Authoritative);
+            Update();
+
+            createdGameObject = CreateAndLinkGameObjectWithComponent<PositionWriterBehaviour>(EntityId);
+            var writer = createdGameObject.GetComponent<PositionWriterBehaviour>().Writer;
+
+            ConnectionHandler.ChangeAuthority(EntityId, Position.ComponentId, Authority.NotAuthoritative);
+            Update();
+
+            Assert.IsNull(createdGameObject.GetComponent<PositionWriterBehaviour>().Writer);
+            Assert.IsFalse(writer.IsValid);
+        }
+
+        [Test]
+        public void CommandSender_is_disabled_if_entity_removed()
+        {
+            createdGameObject = CreateAndLinkGameObjectWithComponent<CommandSender>(EntityId);
+            var sender = createdGameObject.GetComponent<CommandSender>().Sender;
+
+            ConnectionHandler.RemoveComponent(EntityId, Position.ComponentId);
+            ConnectionHandler.RemoveComponent(EntityId, TestCommands.ComponentId);
+            ConnectionHandler.RemoveEntity(EntityId);
+            Update();
+
+            Assert.IsNull(createdGameObject.GetComponent<CommandSender>().Sender);
+            Assert.IsFalse(sender.IsValid);
+        }
+
+        [Test]
+        public void CommandReceiver_is_disabled_if_loses_auth()
+        {
+            ConnectionHandler.ChangeAuthority(EntityId, TestCommands.ComponentId, Authority.Authoritative);
+            Update();
+
+            createdGameObject = CreateAndLinkGameObjectWithComponent<CommandReceiver>(EntityId);
+            var receiver = createdGameObject.GetComponent<CommandReceiver>().Receiver;
+
+            ConnectionHandler.ChangeAuthority(EntityId, TestCommands.ComponentId, Authority.NotAuthoritative);
+            Update();
+
+            Assert.IsNull(createdGameObject.GetComponent<CommandReceiver>().Receiver);
+            Assert.IsFalse(receiver.IsValid);
+        }
+
+        [Test]
+        public void Only_field_which_lost_constraints_is_invalid()
+        {
+            createdGameObject = CreateAndLinkGameObjectWithComponent<MultipleReaderBehaviour>(EntityId);
+            var component = createdGameObject.GetComponent<MultipleReaderBehaviour>();
+            var position = component.PositionReader;
+            var testCommands = component.TestCommandsReader;
+
+            ConnectionHandler.RemoveComponent(EntityId, TestCommands.ComponentId);
+            Update();
+
+            Assert.IsNull(component.PositionReader);
+            Assert.IsNull(component.TestCommandsReader);
+            Assert.IsFalse(position.IsValid);
+            Assert.IsFalse(testCommands.IsValid);
+        }
+
+        public override void TearDown()
+        {
+            base.TearDown();
+            Object.DestroyImmediate(createdGameObject);
+        }
+
+        private void Update()
+        {
+            ReceiveSystem.Update();
+            RequireLifecycleSystem.Update();
+        }
+
+        private class PositionReaderBehaviour : MonoBehaviour
+        {
+            [Require] public PositionReader Reader;
+        }
+
+        private class PositionWriterBehaviour : MonoBehaviour
+        {
+            [Require] public PositionWriter Writer;
+        }
+
+        private class CommandSender : MonoBehaviour
+        {
+            [Require] public TestCommandsCommandSender Sender;
+        }
+
+        private class CommandReceiver : MonoBehaviour
+        {
+            [Require] public TestCommandsCommandReceiver Receiver;
+        }
+
+        private class MultipleReaderBehaviour : MonoBehaviour
+        {
+            [Require] public PositionReader PositionReader;
+            [Require] public TestCommandsReader TestCommandsReader;
+        }
+    }
+}

test-project/Assets/Generated/Source/improbable/dependentschema/DependentComponentComponentReaderWriter.cs

@@ -102,12 +102,7 @@ public override Subscription<DependentComponentReader> Subscribe(EntityId entity
         public override void Cancel(ISubscription subscription)
         {
             var sub = ((Subscription<DependentComponentReader>) subscription);
-            if (sub.HasValue)
-            {
-                var reader = sub.Value;
-                reader.IsValid = false;
-                reader.RemoveAllCallbacks();
-            }
+            ResetValue(sub);
 
             var subscriptions = entityIdToReaderSubscriptions[sub.EntityId];
             subscriptions.Remove(sub);
@@ -124,7 +119,9 @@ public override void ResetValue(ISubscription subscription)
             var sub = ((Subscription<DependentComponentReader>) subscription);
             if (sub.HasValue)
             {
-                sub.Value.RemoveAllCallbacks();
+                var reader = sub.Value;
+                reader.IsValid = false;
+                reader.RemoveAllCallbacks();
             }
         }
 
@@ -227,12 +224,7 @@ public override Subscription<DependentComponentWriter> Subscribe(EntityId entity
         public override void Cancel(ISubscription subscription)
         {
             var sub = ((Subscription<DependentComponentWriter>) subscription);
-            if (sub.HasValue)
-            {
-                var reader = sub.Value;
-                reader.IsValid = false;
-                reader.RemoveAllCallbacks();
-            }
+            ResetValue(sub);
 
             var subscriptions = entityIdToWriterSubscriptions[sub.EntityId];
             subscriptions.Remove(sub);
@@ -249,7 +241,9 @@ public override void ResetValue(ISubscription subscription)
             var sub = ((Subscription<DependentComponentWriter>) subscription);
             if (sub.HasValue)
             {
-                sub.Value.RemoveAllCallbacks();
+                var reader = sub.Value;
+                reader.IsValid = false;
+                reader.RemoveAllCallbacks();
             }
         }
     }
@@ -270,7 +264,7 @@ public DependentComponent.Component Data
             {
                 if (!IsValid)
                 {
-                    throw new InvalidOperationException("Oh noes!");
+                    throw new InvalidOperationException("Cannot read component data when Reader is not valid.");
                 }
 
                 return EntityManager.GetComponentData<DependentComponent.Component>(Entity);
@@ -283,7 +277,7 @@ public Authority Authority
             {
                 if (!IsValid)
                 {
-                    throw new InvalidOperationException("Oh noes!");
+                    throw new InvalidOperationException("Cannot read authority when Reader is not valid");
                 }
 
                 return ComponentUpdateSystem.GetAuthority(EntityId, DependentComponent.ComponentId);

test-project/Assets/Generated/Source/improbable/dependentschema/DependentDataComponentComponentReaderWriter.cs

@@ -102,12 +102,7 @@ public override Subscription<DependentDataComponentReader> Subscribe(EntityId en
         public override void Cancel(ISubscription subscription)
         {
             var sub = ((Subscription<DependentDataComponentReader>) subscription);
-            if (sub.HasValue)
-            {
-                var reader = sub.Value;
-                reader.IsValid = false;
-                reader.RemoveAllCallbacks();
-            }
+            ResetValue(sub);
 
             var subscriptions = entityIdToReaderSubscriptions[sub.EntityId];
             subscriptions.Remove(sub);
@@ -124,7 +119,9 @@ public override void ResetValue(ISubscription subscription)
             var sub = ((Subscription<DependentDataComponentReader>) subscription);
             if (sub.HasValue)
             {
-                sub.Value.RemoveAllCallbacks();
+                var reader = sub.Value;
+                reader.IsValid = false;
+                reader.RemoveAllCallbacks();
             }
         }
 
@@ -227,12 +224,7 @@ public override Subscription<DependentDataComponentWriter> Subscribe(EntityId en
         public override void Cancel(ISubscription subscription)
         {
             var sub = ((Subscription<DependentDataComponentWriter>) subscription);
-            if (sub.HasValue)
-            {
-                var reader = sub.Value;
-                reader.IsValid = false;
-                reader.RemoveAllCallbacks();
-            }
+            ResetValue(sub);
 
             var subscriptions = entityIdToWriterSubscriptions[sub.EntityId];
             subscriptions.Remove(sub);
@@ -249,7 +241,9 @@ public override void ResetValue(ISubscription subscription)
             var sub = ((Subscription<DependentDataComponentWriter>) subscription);
             if (sub.HasValue)
             {
-                sub.Value.RemoveAllCallbacks();
+                var reader = sub.Value;
+                reader.IsValid = false;
+                reader.RemoveAllCallbacks();
             }
         }
     }
@@ -270,7 +264,7 @@ public DependentDataComponent.Component Data
             {
                 if (!IsValid)
                 {
-                    throw new InvalidOperationException("Oh noes!");
+                    throw new InvalidOperationException("Cannot read component data when Reader is not valid.");
                 }
 
                 return EntityManager.GetComponentData<DependentDataComponent.Component>(Entity);
@@ -283,7 +277,7 @@ public Authority Authority
             {
                 if (!IsValid)
                 {
-                    throw new InvalidOperationException("Oh noes!");
+                    throw new InvalidOperationException("Cannot read authority when Reader is not valid");
                 }
 
                 return ComponentUpdateSystem.GetAuthority(EntityId, DependentDataComponent.ComponentId);

test-project/Assets/Generated/Source/improbable/tests/DependencyTestChildComponentReaderWriter.cs

@@ -102,12 +102,7 @@ public override Subscription<DependencyTestChildReader> Subscribe(EntityId entit
         public override void Cancel(ISubscription subscription)
         {
             var sub = ((Subscription<DependencyTestChildReader>) subscription);
-            if (sub.HasValue)
-            {
-                var reader = sub.Value;
-                reader.IsValid = false;
-                reader.RemoveAllCallbacks();
-            }
+            ResetValue(sub);
 
             var subscriptions = entityIdToReaderSubscriptions[sub.EntityId];
             subscriptions.Remove(sub);
@@ -124,7 +119,9 @@ public override void ResetValue(ISubscription subscription)
             var sub = ((Subscription<DependencyTestChildReader>) subscription);
             if (sub.HasValue)
             {
-                sub.Value.RemoveAllCallbacks();
+                var reader = sub.Value;
+                reader.IsValid = false;
+                reader.RemoveAllCallbacks();
             }
         }
 
@@ -227,12 +224,7 @@ public override Subscription<DependencyTestChildWriter> Subscribe(EntityId entit
         public override void Cancel(ISubscription subscription)
         {
             var sub = ((Subscription<DependencyTestChildWriter>) subscription);
-            if (sub.HasValue)
-            {
-                var reader = sub.Value;
-                reader.IsValid = false;
-                reader.RemoveAllCallbacks();
-            }
+            ResetValue(sub);
 
             var subscriptions = entityIdToWriterSubscriptions[sub.EntityId];
             subscriptions.Remove(sub);
@@ -249,7 +241,9 @@ public override void ResetValue(ISubscription subscription)
             var sub = ((Subscription<DependencyTestChildWriter>) subscription);
             if (sub.HasValue)
             {
-                sub.Value.RemoveAllCallbacks();
+                var reader = sub.Value;
+                reader.IsValid = false;
+                reader.RemoveAllCallbacks();
             }
         }
     }
@@ -270,7 +264,7 @@ public DependencyTestChild.Component Data
             {
                 if (!IsValid)
                 {
-                    throw new InvalidOperationException("Oh noes!");
+                    throw new InvalidOperationException("Cannot read component data when Reader is not valid.");
                 }
 
                 return EntityManager.GetComponentData<DependencyTestChild.Component>(Entity);
@@ -283,7 +277,7 @@ public Authority Authority
             {
                 if (!IsValid)
                 {
-                    throw new InvalidOperationException("Oh noes!");
+                    throw new InvalidOperationException("Cannot read authority when Reader is not valid");
                 }
 
                 return ComponentUpdateSystem.GetAuthority(EntityId, DependencyTestChild.ComponentId);