From 6b5fc801678d4f249883431bb600e586b43df437 Mon Sep 17 00:00:00 2001 From: bd_ Date: Thu, 11 May 2023 21:08:33 +0900 Subject: [PATCH] fix: rename parameters hook interferes with menu installer references --- .../VirtualMenuTests/VirtualMenuTests.cs | 34 ++++++ .../Editor/BuildContext.cs | 8 ++ .../Inspector/Menu/MenuInstallerEditor.cs | 112 ++++++++++-------- .../Editor/Menu/VirtualMenu.cs | 61 +++++++++- .../Editor/MenuInstallHook.cs | 2 +- .../Editor/RenameParametersHook.cs | 44 +++---- 6 files changed, 180 insertions(+), 81 deletions(-) diff --git a/Assets/_ModularAvatar/EditModeTests/VirtualMenuTests/VirtualMenuTests.cs b/Assets/_ModularAvatar/EditModeTests/VirtualMenuTests/VirtualMenuTests.cs index bcb41689..91153fd0 100644 --- a/Assets/_ModularAvatar/EditModeTests/VirtualMenuTests/VirtualMenuTests.cs +++ b/Assets/_ModularAvatar/EditModeTests/VirtualMenuTests/VirtualMenuTests.cs @@ -526,6 +526,40 @@ namespace modular_avatar_tests.VirtualMenuTests Assert.True(assetSet.Contains(serialized_c)); } + [Test] + public void InstallTargetToInstallerToInstaller() + { + var menu_a = Create(); + var menu_b = Create(); + var menu_c = Create(); + menu_c.controls = new List + { + GenerateTestControl() + }; + + var node_a = CreateInstaller("root"); + var item_a = node_a.gameObject.AddComponent(); + + var node_b = CreateInstaller("menu_b"); + node_b.menuToAppend = menu_c; + + var node_c = CreateInstaller("menu_c"); + node_c.installTargetMenu = menu_c; + + node_b.transform.parent = node_a.transform; + item_a.installer = node_b; + + var virtualMenu = new VirtualMenu(menu_a); + virtualMenu.RegisterMenuInstallTarget(item_a); + virtualMenu.RegisterMenuInstaller(node_a); + virtualMenu.RegisterMenuInstaller(node_b); + virtualMenu.RegisterMenuInstaller(node_c); + + virtualMenu.FreezeMenu(); + + var root = virtualMenu.ResolvedMenu[menu_a]; + Assert.AreEqual(1, root.Controls.Count); + } ModularAvatarMenuInstaller CreateInstaller(string name) { diff --git a/Packages/nadena.dev.modular-avatar/Editor/BuildContext.cs b/Packages/nadena.dev.modular-avatar/Editor/BuildContext.cs index ad3ff21e..482087b2 100644 --- a/Packages/nadena.dev.modular-avatar/Editor/BuildContext.cs +++ b/Packages/nadena.dev.modular-avatar/Editor/BuildContext.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using JetBrains.Annotations; using UnityEditor; using UnityEditor.Animations; using UnityEngine; @@ -18,6 +19,13 @@ namespace nadena.dev.modular_avatar.core.editor internal readonly Dictionary ClonedMenus = new Dictionary(); + /// + /// This dictionary overrides the _original contents_ of ModularAvatarMenuInstallers. Notably, this does not + /// replace the source menu for the purposes of identifying any other MAMIs that might install to the same + /// menu asset. + /// + internal readonly Dictionary> PostProcessControls + = new Dictionary>(); public BuildContext(VRCAvatarDescriptor avatarDescriptor) { diff --git a/Packages/nadena.dev.modular-avatar/Editor/Inspector/Menu/MenuInstallerEditor.cs b/Packages/nadena.dev.modular-avatar/Editor/Inspector/Menu/MenuInstallerEditor.cs index 889125d6..b830679c 100644 --- a/Packages/nadena.dev.modular-avatar/Editor/Inspector/Menu/MenuInstallerEditor.cs +++ b/Packages/nadena.dev.modular-avatar/Editor/Inspector/Menu/MenuInstallerEditor.cs @@ -28,6 +28,7 @@ namespace nadena.dev.modular_avatar.core.editor private MenuPreviewGUI _previewGUI; private HashSet _avatarMenus; + private VirtualMenu _virtualMenuCache; private Dictionary> _menuInstallersMap; @@ -45,6 +46,61 @@ namespace nadena.dev.modular_avatar.core.editor private long _cacheSeq = -1; private ImmutableList _cachedTargets = null; + private void CacheMenu() + { + if (VirtualMenu.CacheSequence == _cacheSeq && _cachedTargets != null && _virtualMenuCache != null) return; + + + List> perTarget = new List>(); + + var commonAvatar = FindCommonAvatar(); + if (commonAvatar == null) + { + _cacheSeq = VirtualMenu.CacheSequence; + _cachedTargets = ImmutableList.Empty; + _virtualMenuCache = null; + return; + } + + _virtualMenuCache = VirtualMenu.ForAvatar(commonAvatar); + + foreach (var target in targets) + { + var installer = (ModularAvatarMenuInstaller) target; + + var installTargets = _virtualMenuCache.GetInstallTargetsForInstaller(installer) + .Select(o => (object) o).ToImmutableList(); + if (installTargets.Any()) + { + perTarget.Add(installTargets); + } + else + { + perTarget.Add(ImmutableList.Empty.Add(installer.installTargetMenu)); + } + } + + for (int i = 1; i < perTarget.Count; i++) + { + if (perTarget[0].Count != perTarget[i].Count || + perTarget[0].Zip(perTarget[i], (a, b) => (Resolve(a) != Resolve(b))).Any(differs => differs)) + { + perTarget.Clear(); + perTarget.Add(ImmutableList.Empty); + break; + } + } + + _cacheSeq = VirtualMenu.CacheSequence; + _cachedTargets = perTarget[0]; + + object Resolve(object p0) + { + if (p0 is ModularAvatarMenuInstallTarget target && target != null) return target.transform.parent; + return p0; + } + } + // Interpretation: // : Inconsistent install targets // List of [null]: Install to root @@ -55,56 +111,18 @@ namespace nadena.dev.modular_avatar.core.editor { get { - if (VirtualMenu.CacheSequence == _cacheSeq && _cachedTargets != null) return _cachedTargets; + CacheMenu(); - List> perTarget = new List>(); - - var commonAvatar = FindCommonAvatar(); - if (commonAvatar == null) - { - _cacheSeq = VirtualMenu.CacheSequence; - _cachedTargets = ImmutableList.Empty; - return _cachedTargets; - } - - var virtualMenu = VirtualMenu.ForAvatar(commonAvatar); - - foreach (var target in targets) - { - var installer = (ModularAvatarMenuInstaller) target; - - var installTargets = virtualMenu.GetInstallTargetsForInstaller(installer) - .Select(o => (object) o).ToImmutableList(); - if (installTargets.Any()) - { - perTarget.Add(installTargets); - } - else - { - perTarget.Add(ImmutableList.Empty.Add(installer.installTargetMenu)); - } - } - - for (int i = 1; i < perTarget.Count; i++) - { - if (perTarget[0].Count != perTarget[i].Count || - perTarget[0].Zip(perTarget[i], (a, b) => (Resolve(a) != Resolve(b))).Any(differs => differs)) - { - perTarget.Clear(); - perTarget.Add(ImmutableList.Empty); - break; - } - } - - _cacheSeq = VirtualMenu.CacheSequence; - _cachedTargets = perTarget[0]; return _cachedTargets; + } + } - object Resolve(object p0) - { - if (p0 is ModularAvatarMenuInstallTarget target && target != null) return target.transform.parent; - return p0; - } + private VirtualMenu _virtualMenu + { + get + { + CacheMenu(); + return _virtualMenuCache; } } diff --git a/Packages/nadena.dev.modular-avatar/Editor/Menu/VirtualMenu.cs b/Packages/nadena.dev.modular-avatar/Editor/Menu/VirtualMenu.cs index 8ba5843e..6b96e546 100644 --- a/Packages/nadena.dev.modular-avatar/Editor/Menu/VirtualMenu.cs +++ b/Packages/nadena.dev.modular-avatar/Editor/Menu/VirtualMenu.cs @@ -38,21 +38,47 @@ namespace nadena.dev.modular_avatar.core.editor.menu private readonly ImmutableDictionary> _menuToInstallerMap; + private readonly ImmutableDictionary> + _postProcessControls + = ImmutableDictionary>.Empty; + private readonly VirtualMenuNode _node; private readonly NodeForDelegate _nodeFor; private readonly Action _visitedMenu; private readonly HashSet _visited = new HashSet(); + private Action _currentPostprocessor = _control => { }; + + private class PostprocessorContext : IDisposable + { + private NodeContextImpl _context; + private Action _priorPreprocessor; + + public PostprocessorContext(NodeContextImpl context, Action preprocessor) + { + this._context = context; + this._priorPreprocessor = context._currentPostprocessor; + context._currentPostprocessor = preprocessor; + } + + public void Dispose() + { + _context._currentPostprocessor = _priorPreprocessor; + } + } + public NodeContextImpl( VirtualMenuNode node, NodeForDelegate nodeFor, ImmutableDictionary> menuToInstallerMap, + ImmutableDictionary> postProcessControls, Action visitedMenu ) { _node = node; _nodeFor = nodeFor; _menuToInstallerMap = menuToInstallerMap; + _postProcessControls = postProcessControls; _visitedMenu = visitedMenu; } @@ -72,7 +98,10 @@ namespace nadena.dev.modular_avatar.core.editor.menu { foreach (var installer in installers) { - PushNode(installer); + using (new PostprocessorContext(this, null)) + { + PushNode(installer); + } } } } @@ -101,7 +130,10 @@ namespace nadena.dev.modular_avatar.core.editor.menu } else if (installer.menuToAppend != null) { - PushNode(installer.menuToAppend); + using (new PostprocessorContext(this, _postProcessControls.GetValueOrDefault(installer))) + { + PushNode(installer.menuToAppend); + } } }); } @@ -167,6 +199,10 @@ namespace nadena.dev.modular_avatar.core.editor.menu private Dictionary> _installerToTargetComponent = new Dictionary>(); + private ImmutableDictionary> + _postprocessControlsHooks = + ImmutableDictionary>.Empty; + private Dictionary _resolvedMenu = new Dictionary(); // TODO: immutable? @@ -180,8 +216,16 @@ namespace nadena.dev.modular_avatar.core.editor.menu /// Initializes the VirtualMenu. /// /// The root VRCExpressionsMenu to import - internal VirtualMenu(VRCExpressionsMenu rootMenu) + internal VirtualMenu( + VRCExpressionsMenu rootMenu, + BuildContext context = null + ) { + if (context != null) + { + _postprocessControlsHooks = context.PostProcessControls.ToImmutableDictionary(); + } + if (rootMenu != null) { RootMenuKey = rootMenu; @@ -192,9 +236,12 @@ namespace nadena.dev.modular_avatar.core.editor.menu } } - internal static VirtualMenu ForAvatar(VRCAvatarDescriptor avatar) + internal static VirtualMenu ForAvatar( + VRCAvatarDescriptor avatar, + BuildContext context = null + ) { - var menu = new VirtualMenu(avatar.expressionsMenu); + var menu = new VirtualMenu(avatar.expressionsMenu, context); foreach (var installer in avatar.GetComponentsInChildren(true)) { menu.RegisterMenuInstaller(installer); @@ -278,7 +325,8 @@ namespace nadena.dev.modular_avatar.core.editor.menu _resolvedMenu[RootMenuKey] = RootNode; var rootContext = - new NodeContextImpl(RootNode, NodeFor, menuToInstallerFiltered, m => _visitedMenus.Add(m)); + new NodeContextImpl(RootNode, NodeFor, menuToInstallerFiltered, _postprocessControlsHooks, + m => _visitedMenus.Add(m)); if (RootMenuKey is VRCExpressionsMenu menu) { foreach (var control in menu.controls) @@ -311,6 +359,7 @@ namespace nadena.dev.modular_avatar.core.editor.menu BuildReport.ReportingObject(key as UnityEngine.Object, () => { var context = new NodeContextImpl(node, NodeFor, menuToInstallerFiltered, + _postprocessControlsHooks, m => _visitedMenus.Add(m)); if (key is VRCExpressionsMenu expMenu) { diff --git a/Packages/nadena.dev.modular-avatar/Editor/MenuInstallHook.cs b/Packages/nadena.dev.modular-avatar/Editor/MenuInstallHook.cs index ba6c9f9b..9a4918e1 100644 --- a/Packages/nadena.dev.modular-avatar/Editor/MenuInstallHook.cs +++ b/Packages/nadena.dev.modular-avatar/Editor/MenuInstallHook.cs @@ -48,7 +48,7 @@ namespace nadena.dev.modular_avatar.core.editor } _rootMenu = avatar.expressionsMenu; - var virtualMenu = VirtualMenu.ForAvatar(avatar); + var virtualMenu = VirtualMenu.ForAvatar(avatar, context); avatar.expressionsMenu = virtualMenu.SerializeMenu(asset => { context.SaveAsset(asset); diff --git a/Packages/nadena.dev.modular-avatar/Editor/RenameParametersHook.cs b/Packages/nadena.dev.modular-avatar/Editor/RenameParametersHook.cs index 1b067106..0b4882b5 100644 --- a/Packages/nadena.dev.modular-avatar/Editor/RenameParametersHook.cs +++ b/Packages/nadena.dev.modular-avatar/Editor/RenameParametersHook.cs @@ -173,7 +173,7 @@ namespace nadena.dev.modular_avatar.core.editor { if (installer.menuToAppend != null && installer.enabled) { - ProcessMenu(ref installer.menuToAppend, remaps); + ProcessMenuInstaller(installer, remaps); } break; @@ -208,40 +208,22 @@ namespace nadena.dev.modular_avatar.core.editor } } - private void ProcessMenu(ref VRCExpressionsMenu rootMenu, ImmutableDictionary remaps) + private void ProcessMenuInstaller(ModularAvatarMenuInstaller installer, + ImmutableDictionary remaps) { Dictionary remapped = new Dictionary(); - rootMenu = Transform(rootMenu); + if (installer.menuToAppend == null) return; - VRCExpressionsMenu Transform(VRCExpressionsMenu menu) + _context.PostProcessControls.Add(installer, control => { - if (menu == null) return null; - - if (remapped.TryGetValue(menu, out var newMenu)) return newMenu; - - newMenu = Object.Instantiate(menu); - _context.SaveAsset(newMenu); - remapped[menu] = newMenu; - ClonedMenuMappings.Add(menu, newMenu); - - foreach (var control in newMenu.controls) + control.parameter.name = remap(remaps, control.parameter.name); + foreach (var subParam in control.subParameters) { - control.parameter.name = remap(remaps, control.parameter.name); - foreach (var subParam in control.subParameters) - { - subParam.name = remap(remaps, subParam.name); - } - - if (control.type == VRCExpressionsMenu.Control.ControlType.SubMenu) - { - control.subMenu = Transform(control.subMenu); - } + subParam.name = remap(remaps, subParam.name); } - - return newMenu; - } + }); } private void ProcessAnimator(ref AnimatorController controller, ImmutableDictionary remaps) @@ -494,9 +476,17 @@ namespace nadena.dev.modular_avatar.core.editor // This is generic to simplify remapping parameter driver fields, some of which are 'object's. private T remap(ImmutableDictionary remaps, T x) where T : class + { + bool tmp = false; + return remap(remaps, x, ref tmp); + } + + private T remap(ImmutableDictionary remaps, T x, ref bool anyRemapped) + where T : class { if (x is string s && remaps.TryGetValue(s, out var newS)) { + anyRemapped = true; return (T) (object) newS; }