1

I wrote a relatively small user interface for Unity, whose job is to ease working with parts in a scene, i.e. show/hide/toggle objects to reduce clutter.

Long story short, I refactored it to use their new UI API and also started using commands, this allowed me to halve the code.

Although it's pretty easy to understand once you dig in the code, when you generate a diagram out of it, it looks way more complex than it really is in reality.

Here are a few screens for you to understand:

The actual tool, which is just a bunch of buttons in the end:

enter image description here

It's useful in relieving one to relentlessly click icons at the left of the hierarchy, possibly holding multiple keys at the same time just to isolate parts of the scene:

enter image description here

These are the members, pretty easy to understand: callbacks, initializers, updaters; the commands are lambdas so they further reduce the clutter.

enter image description here

However, looking at it in Code Map makes it incredibly confusing to say the least:

enter image description here

I have the feeling that I should ignore it because it simply is the minimum it must be.

And we're only talking about a 250 LOC class, imagine how more confusing it'd be for larger ones.

One could suggest to create a class to encompass some of the members, but in reality that would only move the messiness to somewhere else, i.e. you select outgoing dependencies in Code Map and you'll get the same spaghetti of references...

Just in case, here's the class, as you can see it's really easy to understand:

internal sealed class SceneHierarchyModel : ScriptableObject
{
    private HierachyState AreHidden { get; set; }

    private HierachyState AreVisible { get; set; }

    private HierachyState AreLocked { get; set; }

    private HierachyState ArePickable { get; set; }

    private GameObject[] RootGameObjects { get; set; } = Array.Empty<GameObject>();

    private bool RootGameObjectsAny { get; set; }

    private GameObject[] Selected { get; set; } = Array.Empty<GameObject>();

    private bool SelectedAny { get; set; }

    private static SceneVisibilityManager Manager => SceneVisibilityManager.instance;

    private void OnEnable()
    {
        EditorApplication.hierarchyChanged += OnHierarchyChanged;
        SceneVisibilityManager.pickingChanged += OnPickingChanged;
        SceneVisibilityManager.visibilityChanged += OnVisibilityChanged;
        Selection.selectionChanged += OnSelectionChanged;

        InitializeCommands();
    }

    private void OnDisable()
    {
        EditorApplication.hierarchyChanged -= OnHierarchyChanged;
        SceneVisibilityManager.pickingChanged -= OnPickingChanged;
        SceneVisibilityManager.visibilityChanged -= OnVisibilityChanged;
        Selection.selectionChanged -= OnSelectionChanged;
    }

    private void OnHierarchyChanged()
    {
        Debug.Log(nameof(OnHierarchyChanged));

        var scene = SceneManager.GetActiveScene();

        RootGameObjects = scene.GetRootGameObjects();

        RootGameObjectsAny =
            RootGameObjects.Length > 0;

        Debug.Log(RootGameObjects.Length);

        UpdateCommands();
    }

    private void OnSelectionChanged()
    {
        Selected = Selection
            .GetTransforms(SelectionMode.Editable | SelectionMode.ExcludePrefab)
            .Select(s => s.gameObject)
            .ToArray();

        SelectedAny = Selected.Length > 0;
    }

    private void OnPickingChanged()
    {
        AreLocked = new HierachyState(
            SceneVisibilityWindowObsolete.AreLocked(Selected, false),
            SceneVisibilityWindowObsolete.AreLocked(Selected, true)
        );

        ArePickable = new HierachyState(
            SceneVisibilityWindowObsolete.ArePickable(Selected, false),
            SceneVisibilityWindowObsolete.ArePickable(Selected, true)
        );

        UpdateCommandsPicking();
    }

    private void OnVisibilityChanged()
    {
        AreHidden = new HierachyState(
            SceneVisibilityWindowObsolete.AreHidden(Selected, false),
            SceneVisibilityWindowObsolete.AreHidden(Selected, true)
        );

        AreVisible = new HierachyState(
            SceneVisibilityWindowObsolete.AreVisible(Selected, false),
            SceneVisibilityWindowObsolete.AreVisible(Selected, true)
        );

        UpdateCommandsVisibility();
    }

    private static void NotifyCanExecuteChanged(params RelayCommand[] commands)
    {
        foreach (var command in commands)
        {
            command.NotifyCanExecuteChanged();
        }
    }

    private void InitializeCommands()
    {
        InitializeCommandsPicking();

        InitializeCommandsVisibility();
    }

    private void InitializeCommandsPicking()
    {
        PickingEnable = new RelayCommand(
            () =>
            {
                Manager.EnablePicking(Selected, false);
                SceneHierarchyHooks.ReloadAllSceneHierarchies();
            },
            () => ArePickable.TopLevel == false
        );

        PickingEnableAll = new RelayCommand(
            () =>
            {
                Manager.EnableAllPicking();
                SceneHierarchyHooks.ReloadAllSceneHierarchies();
            },
            () => ArePickable.Recursive == false
        );

        PickingDisable = new RelayCommand(
            () =>
            {
                Manager.DisablePicking(Selected, false);
                SceneHierarchyHooks.ReloadAllSceneHierarchies();
            },
            () => AreLocked.TopLevel == false
        );

        PickingDisableAll = new RelayCommand(
            () =>
            {
                Manager.DisableAllPicking();
                SceneHierarchyHooks.ReloadAllSceneHierarchies();
            },
            () => AreLocked.Recursive == false
        );

        PickingToggle = new RelayCommand(
            () =>
            {
                foreach (var gameObject in Selected)
                {
                    Manager.TogglePicking(gameObject, false);
                }
                SceneHierarchyHooks.ReloadAllSceneHierarchies();
            },
            () => SelectedAny
        );

        PickingToggleAll = new RelayCommand(
            () => SceneVisibilityWindowObsolete.SetPickingToggle(RootGameObjects, true),
            () => RootGameObjectsAny
        );
    }

    private void InitializeCommandsVisibility()
    {
        VisibilityShow = new RelayCommand(
            () => SceneVisibilityWindowObsolete.SetVisibility(Selected, false, true),
            () => AreVisible.TopLevel == false
        );

        VisibilityShowAll = new RelayCommand(
            () => Manager.ShowAll(),
            () => AreVisible.Recursive == false
        );

        VisibilityHide = new RelayCommand(
            () => SceneVisibilityWindowObsolete.SetVisibility(Selected, false, false),
            () => AreHidden.TopLevel == false
        );

        VisibilityHideAll = new RelayCommand(
            () => Manager.HideAll(),
            () => AreHidden.Recursive == false
        );

        VisibilityToggle = new RelayCommand(
            () => SceneVisibilityWindowObsolete.SetVisibilityToggle(Selected, false),
            () => SelectedAny
        );

        VisibilityToggleAll = new RelayCommand(
            () => SceneVisibilityWindowObsolete.SetVisibilityToggle(RootGameObjects, true),
            () => RootGameObjectsAny
        );
    }

    private void UpdateCommands()
    {
        UpdateCommandsPicking();
        UpdateCommandsVisibility();
    }

    private void UpdateCommandsPicking()
    {
        NotifyCanExecuteChanged(
            PickingEnable,
            PickingEnableAll,
            PickingDisable,
            PickingDisableAll,
            PickingToggle,
            PickingToggleAll
        );
    }

    private void UpdateCommandsVisibility()
    {
        NotifyCanExecuteChanged(
            VisibilityShow,
            VisibilityShowAll,
            VisibilityHide,
            VisibilityHideAll,
            VisibilityToggle,
            VisibilityToggleAll
        );
    }

    private sealed record HierachyState(bool TopLevel, bool Recursive);

    #region Commands

    public RelayCommand PickingEnable { get; private set; }

    public RelayCommand PickingEnableAll { get; private set; }

    public RelayCommand PickingDisable { get; private set; }

    public RelayCommand PickingDisableAll { get; private set; }

    public RelayCommand PickingToggle { get; private set; }

    public RelayCommand PickingToggleAll { get; private set; }

    public RelayCommand VisibilityShow { get; private set; }

    public RelayCommand VisibilityShowAll { get; private set; }

    public RelayCommand VisibilityHide { get; private set; }

    public RelayCommand VisibilityHideAll { get; private set; }

    public RelayCommand VisibilityToggle { get; private set; }

    public RelayCommand VisibilityToggleAll { get; private set; }

    #endregion
}

So my question is the following:

Where to draw the line about ignoring the complexity of some code diagrams?

1 Answer 1

11

You won't find consensus about this, but here are a few guidelines:

  • If generating a diagram is a requirement by the team, then this is merely a box-ticking exercise. Generate the diagram and be done with it. Only consider changing it if someone complains.

  • Remember that all these fancy diagrams are about communicating with teammates. If the current diagram is cluttered, but understandable, just be done with it. Job done. Not everything we build as developers looks clean in the IDE and on a diagram.

  • If you really want to declutter the diagram, then you might need to spend time moving boxes and arrows around. The tool is meant to put the boxes and arrows in a picture and arrange them in some sensible way determined by an algorithm, not by visual appeal.

  • Consider splitting the diagram up into multiple pictures, if possible. This will take a deeper analysis of the code to determine where to group all these boxes and arrows into something understandable and cohesive.

    • I like to look for identifiers in the code that have the same prefix or suffix. They might indicate some cohesive concept around which a diagram can be drawn.

    • Look at the user interface you built and identify logical ways to group the behavior or structure of this user interface, and center your diagrams around those concepts.

    • There are likely many more ways to decompose a large diagram. The point is to look for themes or concepts and reorient the perspective of the diagram to reflect this, and omit details that don't fit.

  • And finally, if the team doesn't demand this, and there is no efficient way to declutter this, simply don't do it and move on. The diagram doesn't make the code easier to understand.

Regarding the diagram in this post, I've seen worse. I think the question to ask yourself is whether this diagram adds any value to the overall understanding of the codebase. When in doubt, ask your teammates. If they don't see value in it, then don't waste any more of your time.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.