fix: rename parameters hook interferes with menu installer references

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<VRCExpressionsMenu>();
+            var menu_b = Create<VRCExpressionsMenu>();
+            var menu_c = Create<VRCExpressionsMenu>();
+            menu_c.controls = new List<VRCExpressionsMenu.Control>
+            {
+                GenerateTestControl()
+            };
+
+            var node_a = CreateInstaller("root");
+            var item_a = node_a.gameObject.AddComponent<ModularAvatarMenuInstallTarget>();
+
+            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)
         {

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<VRCExpressionsMenu, VRCExpressionsMenu> ClonedMenus
             = new Dictionary<VRCExpressionsMenu, VRCExpressionsMenu>();
 
+        /// <summary>
+        /// 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.
+        /// </summary>
+        internal readonly Dictionary<ModularAvatarMenuInstaller, Action<VRCExpressionsMenu.Control>> PostProcessControls
+            = new Dictionary<ModularAvatarMenuInstaller, Action<VRCExpressionsMenu.Control>>();
 
         public BuildContext(VRCAvatarDescriptor avatarDescriptor)
         {

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<VRCExpressionsMenu> _avatarMenus;
+        private VirtualMenu _virtualMenuCache;
 
         private Dictionary<VRCExpressionsMenu, List<ModularAvatarMenuInstaller>> _menuInstallersMap;
 
@@ -45,6 +46,61 @@ namespace nadena.dev.modular_avatar.core.editor
         private long _cacheSeq = -1;
         private ImmutableList<object> _cachedTargets = null;
 
+        private void CacheMenu()
+        {
+            if (VirtualMenu.CacheSequence == _cacheSeq && _cachedTargets != null && _virtualMenuCache != null) return;
+
+
+            List<ImmutableList<object>> perTarget = new List<ImmutableList<object>>();
+
+            var commonAvatar = FindCommonAvatar();
+            if (commonAvatar == null)
+            {
+                _cacheSeq = VirtualMenu.CacheSequence;
+                _cachedTargets = ImmutableList<object>.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<object>.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<object>.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:
         //  <empty> : 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<ImmutableList<object>> perTarget = new List<ImmutableList<object>>();
-
-                var commonAvatar = FindCommonAvatar();
-                if (commonAvatar == null)
-                {
-                    _cacheSeq = VirtualMenu.CacheSequence;
-                    _cachedTargets = ImmutableList<object>.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<object>.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<object>.Empty);
-                        break;
-                    }
-                }
-
-                _cacheSeq = VirtualMenu.CacheSequence;
-                _cachedTargets = perTarget[0];
                 return _cachedTargets;
+            }
+        }
 
-                object Resolve(object p0)
+        private VirtualMenu _virtualMenu
-                {
+        {
-                    if (p0 is ModularAvatarMenuInstallTarget target && target != null) return target.transform.parent;
+            get
-                    return p0;
+            {
-                }
+                CacheMenu();
+                return _virtualMenuCache;
             }
         }
 

Packages/nadena.dev.modular-avatar/Editor/Menu/VirtualMenu.cs

@@ -38,21 +38,47 @@ namespace nadena.dev.modular_avatar.core.editor.menu
         private readonly ImmutableDictionary<object, ImmutableList<ModularAvatarMenuInstaller>>
             _menuToInstallerMap;
 
+        private readonly ImmutableDictionary<ModularAvatarMenuInstaller, Action<VRCExpressionsMenu.Control>>
+            _postProcessControls
+                = ImmutableDictionary<ModularAvatarMenuInstaller, Action<VRCExpressionsMenu.Control>>.Empty;
+
         private readonly VirtualMenuNode _node;
         private readonly NodeForDelegate _nodeFor;
         private readonly Action<VRCExpressionsMenu> _visitedMenu;
         private readonly HashSet<object> _visited = new HashSet<object>();
 
+        private Action<VRCExpressionsMenu.Control> _currentPostprocessor = _control => { };
+
+        private class PostprocessorContext : IDisposable
+        {
+            private NodeContextImpl _context;
+            private Action<VRCExpressionsMenu.Control> _priorPreprocessor;
+
+            public PostprocessorContext(NodeContextImpl context, Action<VRCExpressionsMenu.Control> preprocessor)
+            {
+                this._context = context;
+                this._priorPreprocessor = context._currentPostprocessor;
+                context._currentPostprocessor = preprocessor;
+            }
+
+            public void Dispose()
+            {
+                _context._currentPostprocessor = _priorPreprocessor;
+            }
+        }
+
         public NodeContextImpl(
             VirtualMenuNode node,
             NodeForDelegate nodeFor,
             ImmutableDictionary<object, ImmutableList<ModularAvatarMenuInstaller>> menuToInstallerMap,
+            ImmutableDictionary<ModularAvatarMenuInstaller, Action<VRCExpressionsMenu.Control>> postProcessControls,
             Action<VRCExpressionsMenu> 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<ModularAvatarMenuInstaller, List<ModularAvatarMenuInstallTarget>> _installerToTargetComponent
             = new Dictionary<ModularAvatarMenuInstaller, List<ModularAvatarMenuInstallTarget>>();
 
+        private ImmutableDictionary<ModularAvatarMenuInstaller, Action<VRCExpressionsMenu.Control>>
+            _postprocessControlsHooks =
+                ImmutableDictionary<ModularAvatarMenuInstaller, Action<VRCExpressionsMenu.Control>>.Empty;
+
         private Dictionary<object, VirtualMenuNode> _resolvedMenu = new Dictionary<object, VirtualMenuNode>();
 
         // TODO: immutable?
@@ -180,8 +216,16 @@ namespace nadena.dev.modular_avatar.core.editor.menu
         /// Initializes the VirtualMenu.
         /// </summary>
         /// <param name="rootMenu">The root VRCExpressionsMenu to import</param>
-        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<ModularAvatarMenuInstaller>(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)
                         {

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);

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<string, string> remaps)
+        private void ProcessMenuInstaller(ModularAvatarMenuInstaller installer,
+            ImmutableDictionary<string, string> remaps)
         {
             Dictionary<VRCExpressionsMenu, VRCExpressionsMenu> remapped =
                 new Dictionary<VRCExpressionsMenu, VRCExpressionsMenu>();
 
-            rootMenu = Transform(rootMenu);
+            if (installer.menuToAppend == null) return;
 
-            VRCExpressionsMenu Transform(VRCExpressionsMenu menu)
+            _context.PostProcessControls.Add(installer, control =>
             {
-                if (menu == null) return null;
+                control.parameter.name = remap(remaps, control.parameter.name);
-
+                foreach (var subParam in control.subParameters)
-                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);
+                    subParam.name = remap(remaps, subParam.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);
-                    }
                 }
-
+            });
-                return newMenu;
-            }
         }
 
         private void ProcessAnimator(ref AnimatorController controller, ImmutableDictionary<string, string> 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<T>(ImmutableDictionary<string, string> remaps, T x)
             where T : class
+        {
+            bool tmp = false;
+            return remap(remaps, x, ref tmp);
+        }
+
+        private T remap<T>(ImmutableDictionary<string, string> 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;
             }