From 9b58473ae8864b83387eb2998ab0067a4bc11527 Mon Sep 17 00:00:00 2001 From: Evan Hemsley Date: Tue, 16 Jul 2019 11:17:07 -0700 Subject: [PATCH] engines must declare component reads + validation for Reads and Writes arguments --- encompass-cs/Engine.cs | 61 ++++++++-------- encompass-cs/WorldBuilder.cs | 54 ++++++++++----- encompass-cs/attributes/Emits.cs | 16 ----- encompass-cs/attributes/Reads.cs | 14 +++- encompass-cs/attributes/Writes.cs | 26 +++++++ ....cs => EngineMessageSelfCycleException.cs} | 4 +- ...ion.cs => EngineWriteConflictException.cs} | 4 +- ...adException.cs => IllegalReadException.cs} | 4 +- ...ception.cs => IllegalReadTypeException.cs} | 4 +- .../exceptions/IllegalWriteException.cs | 12 ++++ .../exceptions/IllegalWriteTypeException.cs | 12 ++++ encompass-cs/graph/DirectedGraph.cs | 4 +- test/EngineTest.cs | 23 ++++++- test/WorldBuilderTest.cs | 69 ++++++++++++++++++- 14 files changed, 230 insertions(+), 77 deletions(-) delete mode 100644 encompass-cs/attributes/Emits.cs create mode 100644 encompass-cs/attributes/Writes.cs rename encompass-cs/exceptions/{EngineMutationConflictException.cs => EngineMessageSelfCycleException.cs} (61%) rename encompass-cs/exceptions/{IllegalComponentMutationException.cs => EngineWriteConflictException.cs} (60%) rename encompass-cs/exceptions/{IllegalMessageReadException.cs => IllegalReadException.cs} (63%) rename encompass-cs/exceptions/{IllegalMessageEmitException.cs => IllegalReadTypeException.cs} (63%) create mode 100644 encompass-cs/exceptions/IllegalWriteException.cs create mode 100644 encompass-cs/exceptions/IllegalWriteTypeException.cs diff --git a/encompass-cs/Engine.cs b/encompass-cs/Engine.cs index ba5861c..9c336df 100644 --- a/encompass-cs/Engine.cs +++ b/encompass-cs/Engine.cs @@ -74,37 +74,47 @@ namespace Encompass protected TComponent GetComponentByID(Guid componentID) where TComponent : struct, IComponent { - - if (componentManager.GetComponentTypeByID(componentID) == typeof(TComponent)) + if (!readTypes.Contains(typeof(TComponent))) { - return (TComponent)componentManager.GetComponentByID(componentID); + throw new IllegalReadException("Engine {0} tried to read undeclared Component {1}", this.GetType().Name, typeof(TComponent).Name); } - else + + if (componentManager.GetComponentTypeByID(componentID) != typeof(TComponent)) { throw new ComponentTypeMismatchException("Expected Component to be of type {0} but was actually of type {1}", typeof(TComponent).Name, componentManager.GetComponentTypeByID(componentID).Name); } + + return (TComponent)componentManager.GetComponentByID(componentID); } protected IEnumerable> ReadComponents() where TComponent : struct, IComponent { + if (!readTypes.Contains(typeof(TComponent))) + { + throw new IllegalReadException("Engine {0} tried to read undeclared Component {1}", this.GetType().Name, typeof(TComponent).Name); + } + return componentManager.GetActiveComponentsByType(); } protected ValueTuple ReadComponent() where TComponent : struct, IComponent { + if (!readTypes.Contains(typeof(TComponent))) + { + throw new IllegalReadException("Engine {0} tried to read undeclared Component {1}", this.GetType().Name, typeof(TComponent).Name); + } + return componentManager.GetActiveComponentByType(); } internal void UpdateComponentInWorld(Guid componentID, TComponent newComponent) where TComponent : struct, IComponent { - if (writeTypes.Contains(typeof(TComponent))) + if (!writeTypes.Contains(typeof(TComponent))) { - componentManager.UpdateComponent(componentID, newComponent); - } - else - { - throw new IllegalComponentMutationException("Engine {0} tried to write undeclared Component {1}", this.GetType().Name, typeof(TComponent).Name); + throw new IllegalWriteException("Engine {0} tried to write undeclared Component {1}", this.GetType().Name, typeof(TComponent).Name); } + + componentManager.UpdateComponent(componentID, newComponent); } protected void UpdateComponent(Guid componentID, TComponent newComponentValue) where TComponent : struct, IComponent @@ -114,38 +124,33 @@ namespace Encompass protected void EmitMessage(TMessage message) where TMessage : struct, IMessage { - if (writeTypes.Contains(typeof(TMessage))) + if (!writeTypes.Contains(typeof(TMessage))) { - messageManager.AddMessage(message); - } - else - { - throw new IllegalMessageEmitException("Engine {0} tried to emit undeclared Message {1}", this.GetType().Name, typeof(TMessage).Name); + throw new IllegalWriteException("Engine {0} tried to emit undeclared Message {1}", this.GetType().Name, typeof(TMessage).Name); } + + messageManager.AddMessage(message); + } protected IEnumerable ReadMessages() where TMessage : struct, IMessage { - if (readTypes.Contains(typeof(TMessage))) + if (!readTypes.Contains(typeof(TMessage))) { - return messageManager.GetMessagesByType(); - } - else - { - throw new IllegalMessageReadException("Engine {0} tried to read undeclared Message {1}", this.GetType().Name, typeof(TMessage).Name); + throw new IllegalReadException("Engine {0} tried to read undeclared Message {1}", this.GetType().Name, typeof(TMessage).Name); } + + return messageManager.GetMessagesByType(); } protected bool Some() where TMessage : struct, IMessage { - if (readTypes.Contains(typeof(TMessage))) + if (!readTypes.Contains(typeof(TMessage))) { - return messageManager.GetMessagesByType().Any(); - } - else - { - throw new IllegalMessageReadException("Engine {0} tried to read undeclared Message {1}", this.GetType().Name, typeof(TMessage).Name); + throw new IllegalReadException("Engine {0} tried to read undeclared Message {1}", this.GetType().Name, typeof(TMessage).Name); } + + return messageManager.GetMessagesByType().Any(); } protected void Destroy(Guid entityID) diff --git a/encompass-cs/WorldBuilder.cs b/encompass-cs/WorldBuilder.cs index 6c4edf1..3026171 100644 --- a/encompass-cs/WorldBuilder.cs +++ b/encompass-cs/WorldBuilder.cs @@ -17,8 +17,8 @@ namespace Encompass private readonly DrawLayerManager drawLayerManager; private readonly RenderManager renderManager; - private readonly Dictionary> messageTypeToEmitters = new Dictionary>(); - private readonly Dictionary> messageTypeToReaders = new Dictionary>(); + private readonly Dictionary> typeToEmitters = new Dictionary>(); + private readonly Dictionary> typeToReaders = new Dictionary>(); public WorldBuilder() { @@ -55,38 +55,58 @@ namespace Encompass entityManager.RegisterEntityTracker(engine as IEntityTracker); } - foreach (var emitMessageType in engine.writeTypes) + foreach (var writeType in engine.writeTypes) { - if (!messageTypeToEmitters.ContainsKey(emitMessageType)) + if (!typeToEmitters.ContainsKey(writeType)) { - messageTypeToEmitters.Add(emitMessageType, new HashSet()); + typeToEmitters.Add(writeType, new HashSet()); } - messageTypeToEmitters[emitMessageType].Add(engine); + typeToEmitters[writeType].Add(engine); - if (messageTypeToReaders.ContainsKey(emitMessageType)) + if (typeToReaders.ContainsKey(writeType)) { - foreach (var reader in messageTypeToReaders[emitMessageType]) + foreach (var reader in typeToReaders[writeType]) { - engineGraph.AddEdge(engine, reader); + if (engine == reader) + { + if (writeType.GetInterfaces().Contains(typeof(IMessage))) + { + throw new EngineMessageSelfCycleException("Engine both reads and writes Message {0}", writeType.Name); + } + } + else + { + engineGraph.AddEdge(engine, reader); + } } } } - foreach (var readMessageType in engine.readTypes) + foreach (var readType in engine.readTypes) { - if (!messageTypeToReaders.ContainsKey(readMessageType)) + if (!typeToReaders.ContainsKey(readType)) { - messageTypeToReaders.Add(readMessageType, new HashSet()); + typeToReaders.Add(readType, new HashSet()); } - messageTypeToReaders[readMessageType].Add(engine); + typeToReaders[readType].Add(engine); - if (messageTypeToEmitters.ContainsKey(readMessageType)) + if (typeToEmitters.ContainsKey(readType)) { - foreach (var emitter in messageTypeToEmitters[readMessageType]) + foreach (var emitter in typeToEmitters[readType]) { - engineGraph.AddEdge(emitter, engine); + if (emitter == engine) + { + if (readType.GetInterfaces().Contains(typeof(IMessage))) + { + throw new EngineMessageSelfCycleException("Engine both reads and writes Message {0}", readType.Name); + } + } + else + { + engineGraph.AddEdge(emitter, engine); + } } } } @@ -177,7 +197,7 @@ namespace Encompass string.Join(", ", componentToEngines[componentType].Select((engine) => engine.GetType().Name)); } - throw new EngineMutationConflictException(errorString); + throw new EngineWriteConflictException(errorString); } var engineOrder = new List(); diff --git a/encompass-cs/attributes/Emits.cs b/encompass-cs/attributes/Emits.cs deleted file mode 100644 index 75ab92d..0000000 --- a/encompass-cs/attributes/Emits.cs +++ /dev/null @@ -1,16 +0,0 @@ -using System; -using System.Collections.Generic; - -namespace Encompass -{ - [AttributeUsage(AttributeTargets.Class)] - public class Writes : Attribute - { - public readonly List writeTypes; - - public Writes(params Type[] writeTypes) - { - this.writeTypes = new List(writeTypes); - } - } -} diff --git a/encompass-cs/attributes/Reads.cs b/encompass-cs/attributes/Reads.cs index 0ca3ee6..fbf6a0c 100644 --- a/encompass-cs/attributes/Reads.cs +++ b/encompass-cs/attributes/Reads.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.Linq; +using Encompass.Exceptions; namespace Encompass { @@ -8,9 +10,17 @@ namespace Encompass { public readonly List readTypes; - public Reads(params Type[] readMessageTypes) + public Reads(params Type[] readTypes) { - this.readTypes = new List(readMessageTypes); + foreach (var readType in readTypes) + { + if (!readType.GetInterfaces().Contains(typeof(IMessage)) && !readType.GetInterfaces().Contains(typeof(IComponent))) + { + throw new IllegalReadTypeException("{0} must be a Message or Component", readType.Name); + } + } + + this.readTypes = new List(readTypes); } } } diff --git a/encompass-cs/attributes/Writes.cs b/encompass-cs/attributes/Writes.cs new file mode 100644 index 0000000..5af2c6c --- /dev/null +++ b/encompass-cs/attributes/Writes.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Encompass.Exceptions; + +namespace Encompass +{ + [AttributeUsage(AttributeTargets.Class)] + public class Writes : Attribute + { + public readonly List writeTypes; + + public Writes(params Type[] writeTypes) + { + foreach (var writeType in writeTypes) + { + if (!writeType.GetInterfaces().Contains(typeof(IMessage)) && !writeType.GetInterfaces().Contains(typeof(IComponent))) + { + throw new IllegalWriteTypeException("{0} must be a Message or Component", writeType.Name); + } + } + + this.writeTypes = new List(writeTypes); + } + } +} diff --git a/encompass-cs/exceptions/EngineMutationConflictException.cs b/encompass-cs/exceptions/EngineMessageSelfCycleException.cs similarity index 61% rename from encompass-cs/exceptions/EngineMutationConflictException.cs rename to encompass-cs/exceptions/EngineMessageSelfCycleException.cs index 22bd8ce..b345295 100644 --- a/encompass-cs/exceptions/EngineMutationConflictException.cs +++ b/encompass-cs/exceptions/EngineMessageSelfCycleException.cs @@ -2,9 +2,9 @@ using System; namespace Encompass.Exceptions { - public class EngineMutationConflictException : Exception + public class EngineMessageSelfCycleException : Exception { - public EngineMutationConflictException( + public EngineMessageSelfCycleException( string format, params object[] args ) : base(string.Format(format, args)) { } diff --git a/encompass-cs/exceptions/IllegalComponentMutationException.cs b/encompass-cs/exceptions/EngineWriteConflictException.cs similarity index 60% rename from encompass-cs/exceptions/IllegalComponentMutationException.cs rename to encompass-cs/exceptions/EngineWriteConflictException.cs index dfcd7ae..ab1ac08 100644 --- a/encompass-cs/exceptions/IllegalComponentMutationException.cs +++ b/encompass-cs/exceptions/EngineWriteConflictException.cs @@ -2,9 +2,9 @@ using System; namespace Encompass.Exceptions { - public class IllegalComponentMutationException : Exception + public class EngineWriteConflictException : Exception { - public IllegalComponentMutationException( + public EngineWriteConflictException( string format, params object[] args ) : base(string.Format(format, args)) { } diff --git a/encompass-cs/exceptions/IllegalMessageReadException.cs b/encompass-cs/exceptions/IllegalReadException.cs similarity index 63% rename from encompass-cs/exceptions/IllegalMessageReadException.cs rename to encompass-cs/exceptions/IllegalReadException.cs index fe8fb32..8eb5868 100644 --- a/encompass-cs/exceptions/IllegalMessageReadException.cs +++ b/encompass-cs/exceptions/IllegalReadException.cs @@ -2,9 +2,9 @@ using System; namespace Encompass.Exceptions { - public class IllegalMessageReadException : Exception + public class IllegalReadException : Exception { - public IllegalMessageReadException( + public IllegalReadException( string format, params object[] args ) : base(string.Format(format, args)) { } diff --git a/encompass-cs/exceptions/IllegalMessageEmitException.cs b/encompass-cs/exceptions/IllegalReadTypeException.cs similarity index 63% rename from encompass-cs/exceptions/IllegalMessageEmitException.cs rename to encompass-cs/exceptions/IllegalReadTypeException.cs index 411c829..3bc2512 100644 --- a/encompass-cs/exceptions/IllegalMessageEmitException.cs +++ b/encompass-cs/exceptions/IllegalReadTypeException.cs @@ -2,9 +2,9 @@ using System; namespace Encompass.Exceptions { - public class IllegalMessageEmitException : Exception + public class IllegalReadTypeException : Exception { - public IllegalMessageEmitException( + public IllegalReadTypeException( string format, params object[] args ) : base(string.Format(format, args)) { } diff --git a/encompass-cs/exceptions/IllegalWriteException.cs b/encompass-cs/exceptions/IllegalWriteException.cs new file mode 100644 index 0000000..4c7db90 --- /dev/null +++ b/encompass-cs/exceptions/IllegalWriteException.cs @@ -0,0 +1,12 @@ +using System; + +namespace Encompass.Exceptions +{ + public class IllegalWriteException : Exception + { + public IllegalWriteException( + string format, + params object[] args + ) : base(string.Format(format, args)) { } + } +} diff --git a/encompass-cs/exceptions/IllegalWriteTypeException.cs b/encompass-cs/exceptions/IllegalWriteTypeException.cs new file mode 100644 index 0000000..3c720b6 --- /dev/null +++ b/encompass-cs/exceptions/IllegalWriteTypeException.cs @@ -0,0 +1,12 @@ +using System; + +namespace Encompass.Exceptions +{ + public class IllegalWriteTypeException : Exception + { + public IllegalWriteTypeException( + string format, + params object[] args + ) : base(string.Format(format, args)) { } + } +} diff --git a/encompass-cs/graph/DirectedGraph.cs b/encompass-cs/graph/DirectedGraph.cs index 8f562fa..b60db0d 100644 --- a/encompass-cs/graph/DirectedGraph.cs +++ b/encompass-cs/graph/DirectedGraph.cs @@ -27,7 +27,7 @@ namespace Encompass } protected List _vertices = new List(); - protected Dictionary> _neighbors = new Dictionary>(); + protected Dictionary> _neighbors = new Dictionary>(); public IEnumerable Vertices { get { return _vertices; } } @@ -40,7 +40,7 @@ namespace Encompass if (!VertexExists(vertex)) { _vertices.Add(vertex); - _neighbors.Add(vertex, new List()); + _neighbors.Add(vertex, new HashSet()); } } diff --git a/test/EngineTest.cs b/test/EngineTest.cs index f47781d..21a7ff9 100644 --- a/test/EngineTest.cs +++ b/test/EngineTest.cs @@ -17,6 +17,7 @@ namespace Tests static List resultMessages; + [Reads(typeof(MockComponent))] public class ReadComponentsTestEngine : Engine { public override void Update(double dt) @@ -25,6 +26,7 @@ namespace Tests } } + [Reads(typeof(MockComponent))] public class ReadComponentTestEngine : Engine { public override void Update(double dt) @@ -110,6 +112,7 @@ namespace Tests Assert.Throws(() => world.Update(0.01f)); } + [Reads(typeof(MockComponent))] [Writes(typeof(MockComponent))] public class UpdateComponentTestEngine : Engine { @@ -147,6 +150,7 @@ namespace Tests Assert.AreEqual("blaze it", resultComponent.myString); } + [Reads(typeof(MockComponent))] public class UndeclaredUpdateComponentTestEngine : Engine { public override void Update(double dt) @@ -177,7 +181,7 @@ namespace Tests var world = worldBuilder.Build(); - var ex = Assert.Throws(() => world.Update(0.01f)); + var ex = Assert.Throws(() => world.Update(0.01f)); Assert.That(ex.Message, Is.EqualTo("Engine UndeclaredUpdateComponentTestEngine tried to write undeclared Component MockComponent")); } @@ -263,7 +267,7 @@ namespace Tests var world = worldBuilder.Build(); - var ex = Assert.Throws(() => world.Update(0.01f)); + var ex = Assert.Throws(() => world.Update(0.01f)); Assert.That(ex.Message, Is.EqualTo("Engine UndeclaredMessageEmitEngine tried to emit undeclared Message MockMessage")); } @@ -321,12 +325,13 @@ namespace Tests var world = worldBuilder.Build(); - Assert.Throws(() => world.Update(0.01f)); + Assert.Throws(() => world.Update(0.01f)); } static ValueTuple pairA; static ValueTuple pairB; + [Reads(typeof(MockComponent))] class SameValueComponentReadEngine : Engine { public override void Update(double dt) @@ -366,6 +371,7 @@ namespace Tests static IEnumerable> emptyComponentReadResult; + [Reads(typeof(MockComponent))] class ReadEmptyMockComponentsEngine : Engine { public override void Update(double dt) @@ -388,6 +394,7 @@ namespace Tests struct DestroyerComponent : IComponent { } + [Reads(typeof(DestroyerComponent))] class DestroyerEngine : Engine { public override void Update(double dt) @@ -402,6 +409,8 @@ namespace Tests } static IEnumerable> results; + + [Reads(typeof(MockComponent))] class ReaderEngine : Engine { public override void Update(double dt) @@ -439,6 +448,7 @@ namespace Tests Assert.That(results, Does.Not.Contain((componentBID, mockComponent))); } + [Reads(typeof(DestroyerComponent))] class DestroyAndAddComponentEngine : Engine { public override void Update(double dt) @@ -472,6 +482,8 @@ namespace Tests } static Entity entityFromComponentIDResult; + + [Reads(typeof(MockComponent))] class GetEntityFromComponentIDEngine : Engine { public override void Update(double dt) @@ -501,6 +513,8 @@ namespace Tests } static MockComponent mockComponentByIDResult; + + [Reads(typeof(MockComponent))] class GetComponentByIDEngine : Engine { public override void Update(double dt) @@ -530,6 +544,7 @@ namespace Tests struct OtherComponent : IComponent { } + [Reads(typeof(MockComponent), typeof(OtherComponent))] class GetComponentByIDWithTypeMismatchEngine : Engine { public override void Update(double dt) @@ -559,6 +574,8 @@ namespace Tests struct EntityIDComponent : IComponent { public Guid entityID; } static bool hasEntity; + + [Reads(typeof(EntityIDComponent))] class HasEntityTestEngine : Engine { public override void Update(double dt) diff --git a/test/WorldBuilderTest.cs b/test/WorldBuilderTest.cs index 3267281..0519d4e 100644 --- a/test/WorldBuilderTest.cs +++ b/test/WorldBuilderTest.cs @@ -133,7 +133,74 @@ namespace Tests worldBuilder.AddEngine(new AEngine()); worldBuilder.AddEngine(new BEngine()); - Assert.Throws(() => worldBuilder.Build()); + Assert.Throws(() => worldBuilder.Build()); + } + } + + public class EngineMessageSelfCycle + { + struct AMessage : IMessage { } + + [Reads(typeof(AMessage))] + [Writes(typeof(AMessage))] + class AEngine : Engine + { + public override void Update(double dt) + { + + } + } + + [Test] + public void ThrowsError() + { + var worldBuilder = new WorldBuilder(); + + Assert.Throws(() => worldBuilder.AddEngine(new AEngine()), "Engine both reads and writes Message AMessage"); + } + } + + public class IllegalReadType + { + struct ANonMessage { } + + [Reads(typeof(ANonMessage))] + class MyEngine : Engine + { + public override void Update(double dt) + { + + } + } + + [Test] + public void ThrowsError() + { + var worldBuilder = new WorldBuilder(); + + Assert.Throws(() => worldBuilder.AddEngine(new MyEngine()), "ANonMessage must be a Message or Component"); + } + } + + public class IllegalWriteType + { + struct ANonMessage { } + + [Writes(typeof(ANonMessage))] + class MyEngine : Engine + { + public override void Update(double dt) + { + + } + } + + [Test] + public void ThrowsError() + { + var worldBuilder = new WorldBuilder(); + + Assert.Throws(() => worldBuilder.AddEngine(new MyEngine()), "ANonMessage must be a Message or Component"); } }