-1

I'm designing a menu system in Unity and need help validating my approach to ensure it follows the SOLID principles, particularly the Liskov Substitution Principle.
The system consists of:

  • A BaseMenu abstract class for shared functionality.
  • Two specific types of menus:
    • StaticMenu: Predefined buttons assigned in the Unity Editor.
    • DynamicMenu: Buttons created dynamically at runtime based on game context (e.g., dialogue choices).

My Questions:

  1. Does my BaseMenu design properly support the needs of both StaticMenu and DynamicMenu?
  2. Are there any potential violations of SOLID principles in this setup?
  3. Is this the correct approach, or should I consider using an interface or another pattern?

BaseMenu as abstract class

public abstract class BaseMenu
{
    public bool IsActive { get; set; }
    public RectTransform Panel { get; protected set; }
    public List<Button> Buttons { get; protected set; }
    public int CurrentIndex { get; protected set; }

    public virtual void Initialize()
    {
        CurrentIndex = 0;
        Buttons = new List<Button>(); // Ensures Buttons is always initialized
        SetupButtons(); // Abstract method to allow flexibility
    }

    public virtual void Show()
    {
        Panel.gameObject.SetActive(true);
        IsActive = true;
        RegisterWithMenuManager();
    }

    public virtual void Hide()
    {
        Panel.gameObject.SetActive(false);
        IsActive = false;
        DeregisterWithMenuManager();
    }

    public virtual void SelectNext()
    {
        CurrentIndex = (CurrentIndex + 1) % Buttons.Count;
        // Add visual selection logic here
    }

    public virtual void SelectPrevious()
    {
        CurrentIndex = (CurrentIndex - 1 + Buttons.Count) % Buttons.Count;
        // Add visual selection logic here
    }

    public abstract void HandleInput(); // Subclass defines input logic (e.g., vertical vs horizontal)
    protected abstract void SetupButtons(); // Subclass defines how buttons are set up

    public void RegisterWithMenuManager()
    {
        MenuManager.Instance.Register(this); // Add this menu to a stack of active menus
    }

    public void DeregisterWithMenuManager()
    {
        MenuManager.Instance.Deregister(this); // Remove this menu from the stack
    }
}

StaticMenu Implementation
Uses predefined buttons dragged into a SerializedField in the Unity Editor.

public class StaticMenu : BaseMenu
{
    [SerializeField] private List<Button> serializedButtons;

    protected override void SetupButtons()
    {
        Buttons = serializedButtons; // Use predefined buttons
    }

    public override void HandleInput()
    {
        if (Input.GetKeyDown(KeyCode.Up))
        {
            SelectPrevious();
        }
        else if (Input.GetKeyDown(KeyCode.Down))
        {
            SelectNext();
        }
    }
}

DynamicMenu Implementation
Dynamically populates buttons based on a list of options at runtime.

public class DynamicMenu : BaseMenu
{
    private List<string> options;

    public DynamicMenu(List<string> options)
    {
        this.options = options;
    }

    protected override void SetupButtons()
    {
        foreach (var option in options)
        {
            Button newButton = InstantiateButton(option);
            Buttons.Add(newButton);
        }
    }

    private Button InstantiateButton(string text)
    {
        GameObject buttonPrefab = Resources.Load<GameObject>("ButtonPrefab");
        Button button = Instantiate(buttonPrefab, Panel).GetComponent<Button>();
        button.GetComponentInChildren<Text>().text = text;
        return button;
    }

    public override void HandleInput()
    {
        if (Input.GetKeyDown(KeyCode.Up))
        {
            SelectPrevious();
        }
        else if (Input.GetKeyDown(KeyCode.Down))
        {
            SelectNext();
        }
    }
}

I'm new to Unity and SOLID, so feedback is welcome. Thank you!

9
  • 2
    Instead of the initialize method why not rather go by the constructor? In particular if Initialize is virtual any child class could just not call the base implementation and therefore miss things Commented Nov 22, 2024 at 8:14
  • 1
    what Hugo said. also, you can initialize auto-implemented properties on the same line as declaration public List<Button> Buttons { get; protected set; } = new(); and all there is left is to SetupButtons() which you can call in the constructor Commented Nov 22, 2024 at 9:21
  • 1
    The code seems to follow the S of SOLID. The O principle isn't applicable here as it is mostly for changing already existing code. The L is already explained in an answer. The I principle applies to how this code is used by other classes, or how this code uses other classes. E.g. MenuManager can break the I, if its interface exposes more methods than just Register/ Deregister. It also breaks the D: in general, you can't achieve the D principle with singletons. Btw Input, Button, Resources etc also do break the I & the D principles, but it is not practical to abstract Unity APIs. Commented Nov 22, 2024 at 17:05
  • 2
    @Vladimir indeed I often wrap Unity systems in custom wrappers in order to fit them into my architecture .. for some it is really hard to properly separate them into a more MVP like structure though. I often end up having the same Unity built-in thing wrapped at two different places.. seems weird at first but really helps in long term Commented Nov 22, 2024 at 20:02
  • 1
    @derHugo I totally agree that creating wrappers is useful. But there are usually so many Unity APIs used in the project, that trying to abstract them all in a pursue of a super clean architecture is usually too time consuming. Personally, I prefer to not abstract anything used in the View layer, and only abstract the classes that should be accessed by the Model. Commented Nov 22, 2024 at 20:55

1 Answer 1

2

The short answer is that the proposed design breaks the Liskov Substitution Principle (LSP), but there are many other issues, too. Let's stick to the LSP, though, in order to keep focus.

One of the rules of the LSP is that invariants may not be weakened by a subtype.

In order to demonstrate that the design breaks the LSP, only a single counterexample is required.

While the OP shows two subtypes, when you have an abstract class, you allow any number of subtypes. This is also strongly implied by another SOLID principle, the Open Closed Principle.

It's up to the designer of a class to define its contract, which includes preconditions, invariants, and postconditions. The code shown, however, defines at least one invariant:

Buttons = new List<Button>(); // Ensures Buttons is always initialized

The comment indicates that Buttons should always be a proper object. This is a good practice, because otherwise you risk running into null-reference exceptions. Let's assume, for the sake of argument, then, that this is an invariant.

The code allows subclasses to set the Buttons property:

public List<Button> Buttons { get; protected set; }

An new subclass could set Buttons to null, thereby violating the invariant. This, then, violates the LSP.

There are other issues with the design, such as the existence of an Initialize method, which gives rise to sequential coupling.


When considering the LSP, first consider what the implied contract may be. It helps explicitly to write down all preconditions, invariants, and postconditions. Then consider whether these are properly protected by the class.

You're the designer of the class, so only you can define what those are. The more rules you can list, however, the clearer the responsibility of the class is. The fewer you can enumerate, the vaguer the design is.

Here's just one more example for your consideration: Can any subclass set CurrentIndex to -42? Would that be a valid value?

Sign up to request clarification or add additional context in comments.

1 Comment

Thank you for your thoughtful comments, it did point me in the right direction and I believe my latest refactor has simplified and highlighted what was important about what I was trying to accomplish. I've added one of your books to my reading list as well. Cheers.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.