From 48b7d80f7c8cd8e70dbba15b14ea324abdc3ceab Mon Sep 17 00:00:00 2001 From: nekobako Date: Mon, 16 Sep 2024 11:32:59 +0900 Subject: [PATCH] Fix menu item float value (#1140) * fix: menu item with float value incorrectly generates bool parameter * fix: reactive components generate transitions with overlapping condition ranges * chore: add tests for menu item parameter type * fix: incorrect parameter type detemination for float values * chore: add more tests for menu item parameter type * refactor: unify logic to determine parameter type and rename confusing variable --- .../ParamsUsage/MAParametersIntrospection.cs | 11 +-- .../ReactiveObjects/ParameterAssignerPass.cs | 23 ++--- Runtime/ModularAvatarMenuItem.cs | 32 +++++++ .../ParameterAssignment/ParameterTypeTests.cs | 85 +++++++++++++++++++ .../ParameterTypeTests.cs.meta | 3 + 5 files changed, 130 insertions(+), 24 deletions(-) create mode 100644 UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs create mode 100644 UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs.meta diff --git a/Editor/ParamsUsage/MAParametersIntrospection.cs b/Editor/ParamsUsage/MAParametersIntrospection.cs index 2388d178..6ff9f720 100644 --- a/Editor/ParamsUsage/MAParametersIntrospection.cs +++ b/Editor/ParamsUsage/MAParametersIntrospection.cs @@ -34,19 +34,10 @@ namespace nadena.dev.modular_avatar.core.editor hidden = true; } - var type = AnimatorControllerParameterType.Bool; - - if (type != AnimatorControllerParameterType.Float && - (_component.Control.value > 1.01 || _component.Control.value < -0.01)) - type = AnimatorControllerParameterType.Int; - - if (Mathf.Abs(Mathf.Round(_component.Control.value) - _component.Control.value) > 0.01f) - type = AnimatorControllerParameterType.Float; - yield return new ProvidedParameter( name, ParameterNamespace.Animator, - _component, PluginDefinition.Instance, type) + _component, PluginDefinition.Instance, _component.AnimatorControllerParameterType) { WantSynced = _component.isSynced, IsHidden = hidden, diff --git a/Editor/ReactiveObjects/ParameterAssignerPass.cs b/Editor/ReactiveObjects/ParameterAssignerPass.cs index 267be91f..eb0e512f 100644 --- a/Editor/ReactiveObjects/ParameterAssignerPass.cs +++ b/Editor/ReactiveObjects/ParameterAssignerPass.cs @@ -115,8 +115,7 @@ namespace nadena.dev.modular_avatar.core.editor var nextValue = 1; - var canBeBool = true; - var canBeInt = true; + var valueType = VRCExpressionParameters.ValueType.Bool; var isSaved = false; var isSynced = false; @@ -137,10 +136,11 @@ namespace nadena.dev.modular_avatar.core.editor } } - if (Mathf.Abs(mami.Control.value - Mathf.Round(mami.Control.value)) > 0.01f) - canBeInt = false; - else - canBeBool &= mami.Control.value is >= 0 and <= 1; + var newValueType = mami.ExpressionParametersValueType; + if (valueType == VRCExpressionParameters.ValueType.Bool || newValueType == VRCExpressionParameters.ValueType.Float) + { + valueType = newValueType; + } isSaved |= mami.isSaved; isSynced |= mami.isSynced; @@ -148,15 +148,10 @@ namespace nadena.dev.modular_avatar.core.editor if (!declaredParams.ContainsKey(paramName)) { - VRCExpressionParameters.ValueType newType; - if (canBeBool) newType = VRCExpressionParameters.ValueType.Bool; - else if (canBeInt) newType = VRCExpressionParameters.ValueType.Int; - else newType = VRCExpressionParameters.ValueType.Float; - var newParam = new VRCExpressionParameters.Parameter { name = paramName, - valueType = newType, + valueType = valueType, saved = isSaved, defaultValue = defaultValue, networkSynced = isSynced @@ -221,8 +216,8 @@ namespace nadena.dev.modular_avatar.core.editor // we basically force-disable any conditions for nonselected menu items and force-enable any for default // menu items. InitialValue = mami.isDefault ? mami.Control.value : -999, - ParameterValueLo = mami.Control.value - 0.5f, - ParameterValueHi = mami.Control.value + 0.5f, + ParameterValueLo = mami.Control.value - 0.005f, + ParameterValueHi = mami.Control.value + 0.005f, DebugReference = mami, }; } diff --git a/Runtime/ModularAvatarMenuItem.cs b/Runtime/ModularAvatarMenuItem.cs index e38023ee..b5fd5d31 100644 --- a/Runtime/ModularAvatarMenuItem.cs +++ b/Runtime/ModularAvatarMenuItem.cs @@ -149,6 +149,38 @@ namespace nadena.dev.modular_avatar.core if (control.subParameters.Length > maxSubParams) control.subParameters = control.subParameters.Take(maxSubParams).ToArray(); } + + internal VRCExpressionParameters.ValueType ExpressionParametersValueType + { + get + { + // 0, 1 + var type = VRCExpressionParameters.ValueType.Bool; + + // 2, 3, ..., (255) + if (Control.value > 1) + { + type = VRCExpressionParameters.ValueType.Int; + } + + // (-1.0), ..., -0.1, 0.1, ..., 0.9 + if (Control.value < 0 || Mathf.Abs(Control.value - Mathf.Round(Control.value)) > 0.01f) + { + type = VRCExpressionParameters.ValueType.Float; + } + + return type; + } + } + + internal AnimatorControllerParameterType AnimatorControllerParameterType + => ExpressionParametersValueType switch + { + VRCExpressionParameters.ValueType.Bool => AnimatorControllerParameterType.Bool, + VRCExpressionParameters.ValueType.Int => AnimatorControllerParameterType.Int, + VRCExpressionParameters.ValueType.Float => AnimatorControllerParameterType.Float, + _ => 0, + }; } } diff --git a/UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs b/UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs new file mode 100644 index 00000000..6a4a45b5 --- /dev/null +++ b/UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs @@ -0,0 +1,85 @@ +using System; +using System.Linq; +using modular_avatar_tests; +using nadena.dev.modular_avatar.core; +using nadena.dev.modular_avatar.core.editor; +using NUnit.Framework; +using UnityEngine; +using VRC.SDK3.Avatars.Components; +using VRC.SDK3.Avatars.ScriptableObjects; + +namespace UnitTests.ReactiveComponent.ParameterAssignment +{ + public class ParameterTypeTests : TestBase + { + [Test] + public void BoolTest() + { + Test(new[] { 0.0f }, VRCExpressionParameters.ValueType.Bool); + Test(new[] { 1.0f }, VRCExpressionParameters.ValueType.Bool); + + Test(new[] { 0.0f, 1.0f }, VRCExpressionParameters.ValueType.Bool); + Test(new[] { 1.0f, 0.0f }, VRCExpressionParameters.ValueType.Bool); + } + + [Test] + public void IntTest() + { + Test(new[] { 2.0f }, VRCExpressionParameters.ValueType.Int); + Test(new[] { 3.0f }, VRCExpressionParameters.ValueType.Int); + + Test(new[] { 0.0f, 1.0f, 2.0f }, VRCExpressionParameters.ValueType.Int); + Test(new[] { 2.0f, 1.0f, 0.0f }, VRCExpressionParameters.ValueType.Int); + + Test(new[] { 253.0f, 254.0f, 255.0f }, VRCExpressionParameters.ValueType.Int); + Test(new[] { 255.0f, 254.0f, 253.0f }, VRCExpressionParameters.ValueType.Int); + } + + [Test] + public void FloatTest() + { + Test(new[] { -1.0f }, VRCExpressionParameters.ValueType.Float); + Test(new[] { -0.1f }, VRCExpressionParameters.ValueType.Float); + Test(new[] { 0.1f }, VRCExpressionParameters.ValueType.Float); + Test(new[] { 0.9f }, VRCExpressionParameters.ValueType.Float); + + Test(new[] { -1.0f, 0.0f, 1.0f }, VRCExpressionParameters.ValueType.Float); + Test(new[] { 1.0f, 0.0f, -1.0f }, VRCExpressionParameters.ValueType.Float); + + Test(new[] { -0.1f, 0.0f, 0.1f }, VRCExpressionParameters.ValueType.Float); + Test(new[] { 0.1f, 0.0f, -0.1f }, VRCExpressionParameters.ValueType.Float); + + Test(new[] { 0.0f, 0.1f, 0.2f }, VRCExpressionParameters.ValueType.Float); + Test(new[] { 0.2f, 0.1f, 0.0f }, VRCExpressionParameters.ValueType.Float); + + Test(new[] { 0.7f, 0.8f, 0.9f }, VRCExpressionParameters.ValueType.Float); + Test(new[] { 0.9f, 0.8f, 0.7f }, VRCExpressionParameters.ValueType.Float); + } + + private void Test(float[] values, VRCExpressionParameters.ValueType expected) + { + var root = CreateRoot("Root"); + var descriptor = root.GetComponent(); + descriptor.expressionParameters = ScriptableObject.CreateInstance(); + descriptor.expressionParameters.parameters = Array.Empty(); + + for (var i = 0; i < values.Length; i++) + { + var obj = CreateChild(root, i.ToString()); + var mami = obj.AddComponent(); + mami.Control = new VRCExpressionsMenu.Control() + { + name = obj.name, + type = VRCExpressionsMenu.Control.ControlType.Toggle, + value = values[i], + parameter = new() { name = "Test" }, + }; + } + + var context = new nadena.dev.ndmf.BuildContext(root, null); + new ParameterAssignerPass().TestExecute(context); + + Assert.AreEqual(expected, descriptor.expressionParameters.parameters.Single().valueType); + } + } +} \ No newline at end of file diff --git a/UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs.meta b/UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs.meta new file mode 100644 index 00000000..e8b62222 --- /dev/null +++ b/UnitTests~/ReactiveComponent/ParameterAssignment/ParameterTypeTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: c7051ac5a4bbe084c9d34c01c523eda3 +timeCreated: 1726276608 \ No newline at end of file