feat: menu name may use string field #1008

hai-vr · 2024-08-14T05:50:12Z

This is a proposal to include a new "label" field in Modular Avatar Menu Item.

Although not specified, menu items may contain line breaks and rich text, which will be displayed in the radial menu.

It is currently really awkward to write rich text within the object name. In addition to being long, rich text tags will contain a forward slash / and other symbols.

Exposing that field may enable other inventory automation tools to more precisely control the generation of the expression menu items.

This PR attempts to add the following:

Adds a new serializable field "label" in the Modular Avatar Menu Item.
When this "label" field is not an empty string, then Modular Avatar Menu Item would use that instead of the GameObject name.
In the UI, add some means to enable that field for editing, add a multi-line field for that label, and a rich text preview area.

The UI method is currently unsatisfactory, I do not know what would be a good fit. Most users will not use rich text, so the current proposal hides the existence of that new field to the user unless they click a "Rich text" checkbox, which temporarily enables the visibility of the field even when that field is an empty string.

bdunderscore · 2024-08-18T19:28:45Z

Perhaps a better way to handle this would be to have the label default to the gameobject name when the field is empty?
To help make this behavior clearer, we could display the gameobject name in greyed-out text over the label field when not set.

bdunderscore · 2024-08-19T01:57:21Z

We could also only show the preview if the text contains <...

bdunderscore · 2024-09-03T23:07:52Z

Editor/Inspector/Menu/MenuItemGUI.cs

+            if (!needsRichLabel)
+            {
+                EditorGUI.BeginChangeCheck();
+                EditorGUILayout.PropertyField(_name, G("menuitem.prop.name"));


If we're linked, we should just show the name property (which can link directly to the GameObject if this is a ModularAvatarMenuItem). If we're not linked, we should show the label field always (and conditionally show the rich text preview).

bdunderscore · 2024-09-03T23:08:40Z

Editor/Inspector/Menu/MenuItemGUI.cs

+            {
+                EditorGUILayout.PropertyField(_prop_label, G("menuitem.prop.name"));
+            }
+            var linkIcon = EditorGUIUtility.IconContent(needsRichLabel ? "UnLinked" : "Linked").image;


This UI class can be used to render VRCExpressionMenu controls out of expression menu assets as well, in this case the link icon doesn't make sense, as there's no GameObject to link to. See the MenuItemCoreGUI(GameObject parameterReference, SerializedProperty _control, Action redraw) constructor for details.

bdunderscore · 2024-09-03T23:08:51Z

Editor/Inspector/Menu/MenuItemGUI.cs

+                EditorGUILayout.PropertyField(_prop_label, G("menuitem.prop.name"));
+            }
+            var linkIcon = EditorGUIUtility.IconContent(needsRichLabel ? "UnLinked" : "Linked").image;
+            var guiIcon = new GUIContent(linkIcon, S(needsRichLabel ? "menuitem.rich_text.toggle_off.tooltip" : "menuitem.rich_text.toggle_on.tooltip"));


We can just enable this when you type a <

Also, this option doesn't directly control rich text, but rather whether we're linking our name to the gameobject, so the translation key (and text) should probably reflect that.

bdunderscore · 2024-09-03T23:14:04Z

Editor/Inspector/Menu/MenuItemGUI.cs

-            if (EditorGUI.EndChangeCheck())
+
+            EditorGUILayout.BeginHorizontal();
+            var needsRichLabel = (!string.IsNullOrEmpty(_prop_label.stringValue) || _isTryingRichLabel);


Would it be simpler to just do _isTryingRichLabel |= !string.IsNullOrEmpty(_prop_label.stringValue)?

bdunderscore · 2024-09-03T23:14:27Z

Editor/Inspector/Menu/MenuItemGUI.cs

-            EditorGUI.BeginChangeCheck();
-            EditorGUILayout.PropertyField(_name, G("menuitem.prop.name"));
-            if (EditorGUI.EndChangeCheck())
+


One note: Please make sure this doesn't break in some terrible way when multiple items are selected...

I'm stuck on this, I can't figure out how to properly handle multi-editing. Best I can do right now is to maintain the current behaviour where it just edits the GameObject name

It's maybe better to multi-edit the label field in that case. Seems like less surprising behavior than editing the name of all the GameObjects at once?

Should be current behaviour as of the latest commit

hai-vr · 2024-09-05T09:41:53Z

For the next commit, I had to rebase to 1.10.0-rc.4, because it was not possible to test Sub Menu of source Expressions Menu Asset on the base I started on

- Add a link/unlink icon to the right of the name field. - Only show the rich text preview when there is the character '<' in the label field. - Toggling link from ON to OFF will set the label to the current GameObject name.

- Rebased to 1.10.0-rc.4 because the inspector of SubMenu of source Expressions Menu were broken in the base commit this branch initially started with, which was preventing testing some aspects raised during review. - When this is being rendered as part of an SubMenu of source Expressions Menu, don't use any of the label logic, as menu items within such an Expressions Menu are not backed by any GameObject. - Rename _isTryingRichLabel to _useLabel. - Since switching to unlinked always overwrites the label field with the current ObjectName, and switching to linked always empties the label field, the state of _useLabel while the Inspector is open is implied by the value of the label field, or the previous state of the _useLabel field itself when the label field is being emptied out. - In addition, use the |= operator. - When the name is linked, and the user begins typing the "<" character, set the label field, and do not apply the name. This will automatically switch to linked mode as the inspector will be reevaluated a second time. - If the original object name already contains a "<" character (i.e. it comes from a previous version of Modular Avatar), there will be no automatic conversion happening as long as the object name still contains the "<" character. - Changed the localization keys to discard the rich text toggle aspect. - Not addressed: When multiple Menu Item components are selected, the behaviour of the inspector currently edits the GameObject name, with no link button, and no automatic conversion when typing "<", regardless of the contents of the label field.

- Rebased to 1.10.0 - When editing multiple objects, always edit the label.

Spokeek · 2024-10-12T21:19:10Z

I like that. Great idea !

bdunderscore

Thanks for the patch! I'll include this in the next minor update (1.11) - probably in the next week or so?

Note that there's a minor bit of UI weirdness that might be good to fix while waiting - see my comment below.

bdunderscore · 2024-10-12T23:59:21Z

Editor/Inspector/Menu/MenuItemGUI.cs

+                if (GUILayout.Button(guiIcon, GUILayout.Height(EditorGUIUtility.singleLineHeight), GUILayout.Width(25)))
+                {
+                    _prop_label.stringValue = !_useLabel ? _name.stringValue : "";
+                    _useLabel = !_useLabel;
+                }


There's some confusing behavior that occurs when you do the following:

Unlink a menu item

Update its name

Relink it by clicking the link button

Expected behavior: The displayed name in the inspector should match the game object name.

Actual behavior: The displayed name in the inspector is the old label value, until you change focus away from the name field (at which point it'll reset to the gameobject name).

bdunderscore requested changes Sep 3, 2024

View reviewed changes

hai-vr force-pushed the menu-richtext branch from 3622606 to e4d7d8b Compare September 6, 2024 13:03

hai-vr added 4 commits October 1, 2024 09:52

feat: menu name may use string field

b01b280

review:

776f08b

- Add a link/unlink icon to the right of the name field. - Only show the rich text preview when there is the character '<' in the label field. - Toggling link from ON to OFF will set the label to the current GameObject name.

review:

94002e4

- Rebased to 1.10.0 - When editing multiple objects, always edit the label.

hai-vr force-pushed the menu-richtext branch from e4d7d8b to 94002e4 Compare October 1, 2024 19:29

hai-vr marked this pull request as ready for review October 1, 2024 19:33

bdunderscore added the requires minor version bump label Oct 12, 2024

bdunderscore reviewed Oct 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: menu name may use string field #1008

feat: menu name may use string field #1008

hai-vr commented Aug 14, 2024

bdunderscore commented Aug 18, 2024

bdunderscore commented Aug 19, 2024

bdunderscore Sep 3, 2024

bdunderscore Sep 3, 2024

bdunderscore Sep 3, 2024

bdunderscore Sep 3, 2024

bdunderscore Sep 3, 2024

bdunderscore Sep 3, 2024

hai-vr Sep 5, 2024

bdunderscore Sep 14, 2024

hai-vr Oct 1, 2024

hai-vr commented Sep 5, 2024

Spokeek commented Oct 12, 2024

bdunderscore left a comment

bdunderscore Oct 12, 2024

feat: menu name may use string field #1008

Are you sure you want to change the base?

feat: menu name may use string field #1008

Conversation

hai-vr commented Aug 14, 2024

bdunderscore commented Aug 18, 2024

bdunderscore commented Aug 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hai-vr commented Sep 5, 2024

Spokeek commented Oct 12, 2024

bdunderscore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment