From 1e15fc2a6f0e69eacfddbd576b4a9e8db392094a Mon Sep 17 00:00:00 2001 From: bd_ Date: Sun, 22 Sep 2024 17:51:58 -0700 Subject: [PATCH] fix: incorrect auto parameter assignment when a non-auto item is set to 0 --- .../ReactiveObjects/ParameterAssignerPass.cs | 26 ++++++++++++++----- .../AutoValueAssignmentTests.cs | 9 +++++-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/Editor/ReactiveObjects/ParameterAssignerPass.cs b/Editor/ReactiveObjects/ParameterAssignerPass.cs index eb0e512f..9b891f84 100644 --- a/Editor/ReactiveObjects/ParameterAssignerPass.cs +++ b/Editor/ReactiveObjects/ParameterAssignerPass.cs @@ -87,14 +87,15 @@ namespace nadena.dev.modular_avatar.core.editor foreach (var (paramName, list) in _mamiByParam) { // Assign automatic values first - float defaultValue; + int? defaultValue = null; if (declaredParams.TryGetValue(paramName, out var p)) { - defaultValue = p.defaultValue; + defaultValue = (int) p.defaultValue; } else { - defaultValue = list.FirstOrDefault(m => m.isDefault && !m.automaticValue)?.Control?.value ?? 0; + var floatDefault = list.FirstOrDefault(m => m.isDefault && !m.automaticValue)?.Control?.value; + if (floatDefault.HasValue) defaultValue = (int) floatDefault.Value; if (list.Count == 1 && list[0].isDefault && list[0].automaticValue) // If we have only a single entry, it's probably an on-off toggle, so we'll implicitly let 1 @@ -103,7 +104,7 @@ namespace nadena.dev.modular_avatar.core.editor } HashSet usedValues = new(); - usedValues.Add((int)defaultValue); + if (defaultValue.HasValue) usedValues.Add(defaultValue.Value); foreach (var item in list) { @@ -113,6 +114,19 @@ namespace nadena.dev.modular_avatar.core.editor } } + if (!defaultValue.HasValue) + { + for (int i = 0; i < 256; i++) + { + if (!usedValues.Contains(i)) + { + defaultValue = i; + usedValues.Add(i); + break; + } + } + } + var nextValue = 1; var valueType = VRCExpressionParameters.ValueType.Bool; @@ -125,7 +139,7 @@ namespace nadena.dev.modular_avatar.core.editor { if (mami.isDefault) { - mami.Control.value = defaultValue; + mami.Control.value = defaultValue.GetValueOrDefault(); } else { @@ -153,7 +167,7 @@ namespace nadena.dev.modular_avatar.core.editor name = paramName, valueType = valueType, saved = isSaved, - defaultValue = defaultValue, + defaultValue = defaultValue.GetValueOrDefault(), networkSynced = isSynced }; newParameters[paramName] = newParam; diff --git a/UnitTests~/ReactiveComponent/ParameterAssignment/AutoValueAssignmentTests.cs b/UnitTests~/ReactiveComponent/ParameterAssignment/AutoValueAssignmentTests.cs index ce0bd961..d7eb71f5 100644 --- a/UnitTests~/ReactiveComponent/ParameterAssignment/AutoValueAssignmentTests.cs +++ b/UnitTests~/ReactiveComponent/ParameterAssignment/AutoValueAssignmentTests.cs @@ -48,13 +48,18 @@ namespace UnitTests.ReactiveComponent.ParameterAssignment { TestAssignments(new[] { (false, 2.0f), (true, 0.0f), (true, 0.0f) }, new[] { 2.0f, 1.0f, 3.0f }, null); TestAssignments(new[] { (false, 2.7f), (true, 0.0f), (true, 0.0f) }, new[] { 2.7f, 1.0f, 3.0f }, null); + TestAssignments(new[] { (true, 1.0f), (false, 0.0f) }, new[] { 2.0f, 0.0f }, null, overrideExpectedDefaultValue: 1.0f); + TestAssignments(new[] { (true, 1.0f), (false, 0.0f) }, new[] { 1.0f, 0.0f }, 0); + TestAssignments(new[] { (true, 1.0f), (false, 0.0f) }, new[] { 1.0f, 0.0f }, 1); + } void TestAssignments( (bool, float)[] assignments, float[] expectedAssignments, int? defaultIndex = null, - Action> customize = null + Action> customize = null, + float? overrideExpectedDefaultValue = null ) { var root = CreateRoot("root"); @@ -103,7 +108,7 @@ namespace UnitTests.ReactiveComponent.ParameterAssignment Assert.AreEqual(expected, mami.Control.value); } - var expectedDefaultValue = defaultIndex.HasValue ? expectedAssignments[defaultIndex.Value] : 0; + var expectedDefaultValue = overrideExpectedDefaultValue ?? (defaultIndex.HasValue ? expectedAssignments[defaultIndex.Value] : 0); Assert.AreEqual(expectedDefaultValue, avDesc.expressionParameters.parameters.Single().defaultValue); } }