Search code examples
c#unity-game-engineoopgame-developmentsolid-principles

Abstract class design for a menu system in Unity: Static vs Dynamic Menus


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!


Solution

  • 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?