diff --git a/src/Artemis.Core.Tests/Conditions/EventConditionTests.cs b/src/Artemis.Core.Tests/Conditions/EventConditionTests.cs index c5bb7edf6..d3358d904 100644 --- a/src/Artemis.Core.Tests/Conditions/EventConditionTests.cs +++ b/src/Artemis.Core.Tests/Conditions/EventConditionTests.cs @@ -79,16 +79,24 @@ public class EventConditionTests : RenderTestBase } [Fact] - public void Play_EventTriggeredBeforeTheConditionExisted_StillPlaysTheLayer() + public void Play_EventTriggeredBeforeTheConditionExisted_DoesNotPlayTheLayer() { DataModel.Event.Trigger(); (Layer layer, _) = AddLayer(); using RenderFrame frame = Render(layer.Profile); - // Pinned as-is: the condition starts out having seen no triggers at all, so a trigger from before it was created - // (for example long before the profile was activated) still counts as new and plays the layer once. Looks like - // the condition should only react to triggers that happen after it started watching. + ShouldBeOff(frame); + } + + [Fact] + public void Play_EventTriggeredAfterTheConditionExisted_PlaysTheLayer_EvenBeforeTheFirstFrame() + { + (Layer layer, _) = AddLayer(); + DataModel.Event.Trigger(); + + using RenderFrame frame = Render(layer.Profile); + ShouldBeOn(frame); } @@ -418,15 +426,14 @@ public class EventConditionTests : RenderTestBase } [Fact] - public void PathToAValue_PlaysOnTheFirstFrame() + public void PathToAValue_DoesNotPlayOnTheFirstFrame() { (Layer layer, _) = AddLayer(eventPath: "Integer"); using RenderFrame frame = Render(layer.Profile); - // Pinned as-is: the last seen value starts out empty, so the first frame always sees the value as changed (0 is - // not empty) and plays the layer even though nothing changed. - ShouldBeOn(frame); + // The first frame only learns the starting value, nothing changed yet + ShouldBeOff(frame); } [Fact] diff --git a/src/Artemis.Core.Tests/DataModels/DataModelPathTests.cs b/src/Artemis.Core.Tests/DataModels/DataModelPathTests.cs index 44c7eb910..6eb3e1709 100644 --- a/src/Artemis.Core.Tests/DataModels/DataModelPathTests.cs +++ b/src/Artemis.Core.Tests/DataModels/DataModelPathTests.cs @@ -340,7 +340,7 @@ public class DataModelPathTests : IDisposable } [Fact] - public void RemovingTheDynamicChild_LeavesThePathValid() + public void RemovingTheDynamicChild_MakesThePathInvalid() { DataModel.AddDynamicChild("Extra", 5); using DataModelPath path = new(DataModel, "Extra"); @@ -349,25 +349,34 @@ public class DataModelPathTests : IDisposable DataModel.RemoveDynamicChildByKey("Extra"); - // Pinned as-is: the segment only invalidates the path when the value of the removed child is the data model the - // segment looks in, rather than when the child it points to is removed. So the path stays valid and reads the - // default value instead of becoming invalid. - path.IsValid.ShouldBeTrue(); - path.GetValue().ShouldBe(0); - invalidated.ShouldBe(0); + path.IsValid.ShouldBeFalse(); + path.GetValue().ShouldBeNull(); + invalidated.ShouldBe(1); } [Fact] - public void RemovingADynamicDataModel_LeavesThePathThroughItValid() + public void RemovingTheDynamicChild_AndAddingItAgain_MakesThePathValidAgain() + { + DataModel.AddDynamicChild("Extra", 5); + using DataModelPath path = new(DataModel, "Extra"); + DataModel.RemoveDynamicChildByKey("Extra"); + + DataModel.AddDynamicChild("Extra", 6); + + path.IsValid.ShouldBeTrue(); + path.GetValue().ShouldBe(6); + } + + [Fact] + public void RemovingADynamicDataModel_MakesThePathThroughItInvalid() { DataModel.AddDynamicChild("Extra", new TestChildDataModel {Integer = 4}, "Extra data"); using DataModelPath path = new(DataModel, "Extra.Integer"); DataModel.RemoveDynamicChildByKey("Extra"); - // Pinned as-is, see RemovingTheDynamicChild_LeavesThePathValid - path.IsValid.ShouldBeTrue(); - path.GetValue().ShouldBe(0); + path.IsValid.ShouldBeFalse(); + path.GetValue().ShouldBeNull(); } [Fact] @@ -450,20 +459,17 @@ public class DataModelPathTests : IDisposable } [Fact] - public void NewPath_DoesNotFollowTheStoreUntilItIsSaved() + public void NewPath_FollowsTheStoreWithoutBeingSaved() { using DataModelPath path = new(DataModel, "Integer"); - // Pinned as-is: the constructors call Save before the path is set up, while it is still invalid, so nothing is - // saved and the entity has no data model ID. The path only reacts to its data model leaving and returning to the - // store once Save is called. Looks like Save should be called after Initialize. - path.Entity.DataModelId.ShouldBeNull(); - path.Entity.Path.ShouldBe(""); + path.Entity.DataModelId.ShouldBe(_module.Id); + path.Entity.Path.ShouldBe("Integer"); RemoveFromStore(); - path.IsValid.ShouldBeTrue(); - path.Target.ShouldBeSameAs(DataModel); + path.IsValid.ShouldBeFalse(); + path.Target.ShouldBeNull(); } [Fact] @@ -540,7 +546,7 @@ public class DataModelPathTests : IDisposable } [Fact] - public void InvalidPath_CannotFallBackToATypeOutsideTheCoreLibraries() + public void InvalidPath_FallsBackToATypeOutsideTheCoreLibraries() { using DataModelPath valid = new(DataModel, "Enum"); valid.Save(); @@ -548,9 +554,15 @@ public class DataModelPathTests : IDisposable using DataModelPath path = new(new DataModelPathEntity {Path = "Enum", DataModelId = "Missing-12345678", Type = valid.Entity.Type}); - // Pinned as-is: the type is saved by its full name without the assembly, which Type.GetType can only find in - // Artemis.Core and the .NET libraries, so types from plugins (and SkiaSharp, like SKColor) can't be found again. - path.GetPropertyType().ShouldBeNull(); + path.GetPropertyType().ShouldBe(typeof(TestEnum)); + } + + [Fact] + public void InvalidPath_FallsBackToASkiaSharpType() + { + using DataModelPath path = new(new DataModelPathEntity {Path = "Color", DataModelId = "Missing-12345678", Type = typeof(SKColor).FullName}); + + path.GetPropertyType().ShouldBe(typeof(SKColor)); } [Fact] diff --git a/src/Artemis.Core.Tests/DataModels/DataModelTests.cs b/src/Artemis.Core.Tests/DataModels/DataModelTests.cs index 68d002dd9..df4f49416 100644 --- a/src/Artemis.Core.Tests/DataModels/DataModelTests.cs +++ b/src/Artemis.Core.Tests/DataModels/DataModelTests.cs @@ -184,7 +184,7 @@ public class DataModelTests : IDisposable } [Fact] - public void RemovingADynamicChild_DoesNotRemoveIt() + public void RemovingADynamicChild_RemovesIt() { DynamicChild child = DataModel.AddDynamicChild("Extra", 5); int removed = 0; @@ -192,11 +192,8 @@ public class DataModelTests : IDisposable DataModel.RemoveDynamicChild(child); - // Pinned as-is: RemoveDynamicChild compares the value of each child (BaseValue) with the child that was passed in, - // rather than the child itself, so it only removes a child whose value is the child object. Looks like it should - // compare the children themselves; RemoveDynamicChildByKey works. - DataModel.DynamicChildren.ShouldContainKey("Extra"); - removed.ShouldBe(0); + DataModel.DynamicChildren.ShouldNotContainKey("Extra"); + removed.ShouldBe(1); } [Fact] @@ -304,14 +301,14 @@ public class DataModelTests : IDisposable } [Fact] - public void IncludingChildren_AlsoMatchesPropertiesThatStartWithTheSameName() + public void IncludingChildren_DoesNotMatchPropertiesThatStartWithTheSameName() { using DataModelPath path = new(DataModel, "Integers"); - // Pinned as-is: children are matched by checking whether a path starts with the given text, so "Integer" counts as - // in use because of a path to "Integers", a different property. Looks like it should check for "Integer." - _module.IsPropertyInUse("Integer", true).ShouldBeTrue(); + // Integers is a different property than Integer, not a child of it + _module.IsPropertyInUse("Integer", true).ShouldBeFalse(); _module.IsPropertyInUse("Integer", false).ShouldBeFalse(); + _module.IsPropertyInUse("Integers", true).ShouldBeTrue(); } [Fact] diff --git a/src/Artemis.Core/Models/Profile/Conditions/EventCondition.cs b/src/Artemis.Core/Models/Profile/Conditions/EventCondition.cs index bbe50bdc1..acf68a02d 100644 --- a/src/Artemis.Core/Models/Profile/Conditions/EventCondition.cs +++ b/src/Artemis.Core/Models/Profile/Conditions/EventCondition.cs @@ -17,8 +17,10 @@ public class EventCondition : CorePropertyChanged, INodeScriptCondition private NodeScript _script; private DefaultNode _startNode; private DataModelPath? _eventPath; - private DateTime _lastProcessedTrigger; + // Only what happens after the condition starts watching its path counts, see StartWatching + private DateTime _lastProcessedTrigger = DateTime.Now; private object? _lastProcessedValue; + private bool _hasProcessedValue; private EventOverlapMode _overlapMode; private EventToggleOffMode _toggleOffMode; private EventTriggerMode _triggerMode; @@ -64,7 +66,11 @@ public class EventCondition : CorePropertyChanged, INodeScriptCondition public DataModelPath? EventPath { get => _eventPath; - set => SetAndNotify(ref _eventPath, value); + set + { + if (SetAndNotify(ref _eventPath, value)) + StartWatching(); + } } /// @@ -243,6 +249,15 @@ public class EventCondition : CorePropertyChanged, INodeScriptCondition Script.AddNode(newStartNode); } + private void StartWatching() + { + // Triggers and values from before the condition watched the path don't count, otherwise for example an event that + // triggered long before the profile was activated would still play the element once + _lastProcessedTrigger = DateTime.Now; + _lastProcessedValue = null; + _hasProcessedValue = false; + } + private bool Evaluate() { if (EventPath == null) @@ -258,6 +273,14 @@ public class EventCondition : CorePropertyChanged, INodeScriptCondition } else if (_startNode is EventConditionValueChangedStartNode valueChangedNode) { + // The first value seen is where the value starts, not a change + if (!_hasProcessedValue) + { + _lastProcessedValue = value; + _hasProcessedValue = true; + return false; + } + if (Equals(value, _lastProcessedValue)) return false; diff --git a/src/Artemis.Core/Models/Profile/DataModel/DataModelPath.cs b/src/Artemis.Core/Models/Profile/DataModel/DataModelPath.cs index 62ebd93bb..9039e6792 100644 --- a/src/Artemis.Core/Models/Profile/DataModel/DataModelPath.cs +++ b/src/Artemis.Core/Models/Profile/DataModel/DataModelPath.cs @@ -29,8 +29,8 @@ public class DataModelPath : IStorageModel, IDisposable, IPluginFeatureDependent _segments = []; - Save(); Initialize(); + Save(); SubscribeToDataModelStore(); } @@ -47,8 +47,8 @@ public class DataModelPath : IStorageModel, IDisposable, IPluginFeatureDependent _segments = []; - Save(); Initialize(); + Save(); SubscribeToDataModelStore(); } @@ -67,8 +67,8 @@ public class DataModelPath : IStorageModel, IDisposable, IPluginFeatureDependent _segments = []; - Save(); Initialize(); + Save(); SubscribeToDataModelStore(); } @@ -165,11 +165,20 @@ public class DataModelPath : IStorageModel, IDisposable, IPluginFeatureDependent // Fall back to stored type if (!string.IsNullOrWhiteSpace(Entity.Type)) - return Type.GetType(Entity.Type); + return FindType(Entity.Type); return null; } + private static Type? FindType(string fullName) + { + // The type is stored without its assembly, Type.GetType only finds it in Artemis.Core and the .NET libraries. Types + // of plugins and other libraries (like SKColor) are found by looking through the loaded assemblies + return Type.GetType(fullName) ?? AppDomain.CurrentDomain.GetAssemblies() + .Select(assembly => assembly.GetType(fullName)) + .FirstOrDefault(type => type != null); + } + /// /// Gets the property description of the property this path points to /// diff --git a/src/Artemis.Core/Models/Profile/DataModel/DataModelPathSegment.cs b/src/Artemis.Core/Models/Profile/DataModel/DataModelPathSegment.cs index f1c71bd34..c1db971bc 100644 --- a/src/Artemis.Core/Models/Profile/DataModel/DataModelPathSegment.cs +++ b/src/Artemis.Core/Models/Profile/DataModel/DataModelPathSegment.cs @@ -286,8 +286,16 @@ public class DataModelPathSegment : IDisposable private void DynamicChildOnDynamicChildRemoved(object? sender, DynamicDataModelChildEventArgs e) { - if (e.DynamicChild.BaseValue == _dynamicDataModel) + // Like when a child is added, initialize the path again so it no longer points to the removed child + if (e.Key == Identifier) + { DataModelPath.Invalidate(); + DataModelPath.Initialize(); + } + else if (e.DynamicChild.BaseValue == _dynamicDataModel) + { + DataModelPath.Invalidate(); + } } /// diff --git a/src/Artemis.Core/Plugins/Modules/DataModel.cs b/src/Artemis.Core/Plugins/Modules/DataModel.cs index 2d82d8037..12a3e5b97 100644 --- a/src/Artemis.Core/Plugins/Modules/DataModel.cs +++ b/src/Artemis.Core/Plugins/Modules/DataModel.cs @@ -289,7 +289,7 @@ public abstract class DataModel public void RemoveDynamicChild(DynamicChild dynamicChild) { if (dynamicChild == null) throw new ArgumentNullException(nameof(dynamicChild)); - List keys = _dynamicChildren.Where(kvp => kvp.Value.BaseValue == dynamicChild).Select(kvp => kvp.Key).ToList(); + List keys = _dynamicChildren.Where(kvp => kvp.Value == dynamicChild).Select(kvp => kvp.Key).ToList(); foreach (string key in keys) { _dynamicChildren.Remove(key); @@ -335,8 +335,10 @@ public abstract class DataModel { lock (_activePaths) { + // A child's path continues after a dot, matching on the text alone would also match a property that merely starts + // with the same name (Integer and Integers) return includeChildren - ? _activePathsHashSet.Any(p => p.StartsWith(path, PathsStringComparison)) + ? _activePathsHashSet.Any(p => string.Equals(p, path, PathsStringComparison) || p.StartsWith(path + ".", PathsStringComparison)) : _activePathsHashSet.Contains(path); } }