YarnSpinnerTool / YarnSpinner-Unity

The official Unity integration for Yarn Spinner, the friendly dialogue tool.
MIT License
491 stars 85 forks source link

YarnNode Attribute doesn't display dropdown in certain data layouts. #257

Closed unknowndevice closed 7 months ago

unknowndevice commented 9 months ago

What is the current behavior?

Using the YarnNode Attribute doesn't display correctly in Arrays of objects. You can't select a node using the drop down. Instead it looks like this.

Screenshot 2023-11-21 at 21 27 14

Please provide the steps to reproduce, and if possible a minimal demo of the problem:

  1. Create an editor script with the ActivityEditor class below
  2. Create a scriptable script with the Activity script below
  3. Create a scriptable
  4. Click "Add Node Activity" and select Dialogue
  5. Add a Yarn Project with Nodes
  6. You'll now see the red warning with no way to select a node.
#if UNITY_EDITOR

using UnityEditor;
using UnityEngine;
using Yarn.Unity;

namespace VideoGame.Editor
{
    [CustomEditor(typeof(Activity))]
    public class ActivityEditor : UnityEditor.Editor
    {
        public override void OnInspectorGUI()
        {
            DrawDefaultInspector();
            DrawNodesButton();

            if (Application.isPlaying || EditorApplication.isPlaying) return;

            void DrawNodesButton()
            {
                if (!GUILayout.Button("Add Node Activity")) return;

                GenericMenu menu = new();

                AddMenuItem(menu, "Battle", ActivityName.Battle);
                AddMenuItem(menu, "Dialogue", ActivityName.Dialogue);
                AddMenuItem(menu, "Side Activity", ActivityName.SideActivity);

                menu.ShowAsContext();

                void AddMenuItem(GenericMenu menu, string menuPath, ActivityName activityName)
                {
                    menu.AddItem(new GUIContent(menuPath), false, OnSelected, activityName);

                    void OnSelected(object activityName)
                    {
                        var yarn = (YarnProject)serializedObject.FindProperty("YarnProject").objectReferenceValue;
                        var nodes = serializedObject.FindProperty("Nodes");
                        var count = nodes.arraySize;
                        nodes.InsertArrayElementAtIndex(count);
                        var property = nodes.GetArrayElementAtIndex(count);

                        property.managedReferenceValue = (ActivityName)activityName switch {
                            ActivityName.Battle => new Activity.Battle { ActivityName = (ActivityName)activityName },
                            ActivityName.Dialogue => new Activity.Dialogue { ActivityName = (ActivityName)activityName, Node = string.Empty },
                            ActivityName.SideActivity => new Activity.SideActivity { ActivityName = (ActivityName)activityName },
                            _ => new Activity.Dialogue { ActivityName = (ActivityName)activityName, Node = string.Empty },
                        };

                        serializedObject.ApplyModifiedProperties();
                    }
                }
            }
        }
    }
}

#endif

using Assist;
using System;
using System.Collections.Generic;
using UnityEngine;
using Yarn.Unity;

namespace VideoGame
{
    public enum ActivityName { Battle = 0, Dialogue = 1, SideActivity = 2 };

    [CreateAssetMenu(menuName = "Activity")]
    public class Activity : ScriptableObject
    {
        [SerializeReference] public List<AcitivityNode> Nodes;

        [Serializable]
        public abstract class AcitivityNode
        {
#if UNITY_EDITOR
            [ReadOnly] public string Name;
#endif
            public ActivityName ActivityName;

            public abstract bool IsComplete();
        }

        [Serializable]
        public class Dialogue : AcitivityNode
        {
            public YarnProject Project;
            [YarnNode(nameof(Project))] public string Node;

            public override bool IsComplete() => false;

#if UNITY_EDITOR
            public Dialogue() => Name = "Dialogue";
#endif
        }

        [Serializable]
        public class Battle : AcitivityNode
        {
            public bool PlayBattleMusic = true;
            public ScriptableObject battle;

            public override bool IsComplete() => false;

#if UNITY_EDITOR
            public Battle() => Name = "Battle";
#endif
        }

        [Serializable]
        public class SideActivity : AcitivityNode
        {
            public string Node;
            public bool Unrepeatable;

            public override bool IsComplete() => false;

#if UNITY_EDITOR
            public SideActivity() => Name = "Side Activity";
#endif
        }
    }
}

What is the expected behavior?

The YarnNode Attribute dropdown listing all the Nodes should be displayed

Please tell us about your environment:

Other information

McJones commented 7 months ago

I am having a bit of an issue reproducing this in Unity 2022 or 2021.

I had to slightly modify your editor script, in particular the line:

var yarn = (YarnProject)serializedObject.FindProperty("YarnProject").objectReferenceValue;

will always cause a null reference exception because the Activity scriptable object doesn't have a YarnProject property.

Additionally I don't have the Assist include because I am not sure what that is, I assume something internal that wraps about the job system. I am assuming this because you are using [ReadOnly] attributes.

Once I fixed those, by removing the FindProperty call and replacing using Assist; with using Unity.Collections; the node attribute works:

Screen Shot 2024-01-19 at 2 34 30 pm

I am including my slightly tweaked versions of Activity and ActivityEditor:

#if UNITY_EDITOR

using UnityEditor;
using UnityEngine;
using Yarn.Unity;

namespace VideoGame.Editor
{
    [CustomEditor(typeof(Activity))]
    public class ActivityEditor : UnityEditor.Editor
    {
        public override void OnInspectorGUI()
        {
            DrawDefaultInspector();
            DrawNodesButton();

            if (Application.isPlaying || EditorApplication.isPlaying) return;

            void DrawNodesButton()
            {
                if (!GUILayout.Button("Add Node Activity")) return;

                GenericMenu menu = new();

                AddMenuItem(menu, "Battle", ActivityName.Battle);
                AddMenuItem(menu, "Dialogue", ActivityName.Dialogue);
                AddMenuItem(menu, "Side Activity", ActivityName.SideActivity);

                menu.ShowAsContext();

                void AddMenuItem(GenericMenu menu, string menuPath, ActivityName activityName)
                {
                    menu.AddItem(new GUIContent(menuPath), false, OnSelected, activityName);

                    void OnSelected(object activityName)
                    {
                        var nodes = serializedObject.FindProperty("Nodes");
                        var count = nodes.arraySize;
                        nodes.InsertArrayElementAtIndex(count);
                        var property = nodes.GetArrayElementAtIndex(count);

                        property.managedReferenceValue = (ActivityName)activityName switch {
                            ActivityName.Battle => new Activity.Battle { ActivityName = (ActivityName)activityName },
                            ActivityName.Dialogue => new Activity.Dialogue { ActivityName = (ActivityName)activityName, Node = string.Empty },
                            ActivityName.SideActivity => new Activity.SideActivity { ActivityName = (ActivityName)activityName },
                            _ => new Activity.Dialogue { ActivityName = (ActivityName)activityName, Node = string.Empty },
                        };

                        serializedObject.ApplyModifiedProperties();
                    }
                }
            }
        }
    }
}
#endif

using System;
using System.Collections.Generic;
using UnityEngine;
using Unity.Collections;
using Yarn.Unity;

namespace VideoGame
{
    public enum ActivityName { Battle = 0, Dialogue = 1, SideActivity = 2 };

    [CreateAssetMenu(menuName = "Activity")]
    public class Activity : ScriptableObject
    {
        [SerializeReference] public List<AcitivityNode> Nodes;

        [Serializable]
        public abstract class AcitivityNode
        {
#if UNITY_EDITOR
            [ReadOnly] public string Name;
#endif
            public ActivityName ActivityName;

            public abstract bool IsComplete();
        }

        [Serializable]
        public class Dialogue : AcitivityNode
        {
            public YarnProject Project;
            [YarnNode(nameof(Project))] public string Node;

            public override bool IsComplete() => false;

#if UNITY_EDITOR
            public Dialogue() => Name = "Dialogue";
#endif
        }

        [Serializable]
        public class Battle : AcitivityNode
        {
            public bool PlayBattleMusic = true;
            public ScriptableObject battle;

            public override bool IsComplete() => false;

#if UNITY_EDITOR
            public Battle() => Name = "Battle";
#endif
        }

        [Serializable]
        public class SideActivity : AcitivityNode
        {
            public string Node;
            public bool Unrepeatable;

            public override bool IsComplete() => false;

#if UNITY_EDITOR
            public SideActivity() => Name = "Side Activity";
#endif
        }
    }
}

But the issue now is I am no longer sure if what I am doing is actually failing to reproducing your issue or fundamentally changing things and thus side-stepping the issue. Thoughts?

McJones commented 7 months ago

It's been two weeks and there has been no movement on this issue. I think my changes to your sample has fixed it so I am going to close this issue for now. If the issue persists let me know and I can reopen it or you can file a new issue.