From bbaf21c25ff9c2fd1eb5f17ebec9ee9986954f20 Mon Sep 17 00:00:00 2001 From: RobertBeekman Date: Tue, 20 Oct 2026 09:26:46 +0200 Subject: [PATCH] Core - Fix removing dynamic children and checking which properties are in use - DataModel.RemoveDynamicChild compared the value of each child with the child that was passed in, so it never removed anything. - IsPropertyInUse with includeChildren matched paths by their start, so a path to "Integers" made "Integer" count as in use. Children are now matched on what follows a dot. --- .../DataModels/DataModelTests.cs | 17 +++++++---------- src/Artemis.Core/Plugins/Modules/DataModel.cs | 6 ++++-- 2 files changed, 11 insertions(+), 12 deletions(-) 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/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); } }