Как лучше спроектировать индикацию состояния в Unity, C#

Рейтинг: 0Ответов: 2Опубликовано: 28.06.2023

Столкнулся со сверх тривиальной задачей, которая поставила меня в тупик. Есть 4 состояния, и есть некий прибор, который их отображает. В моем случае это классический селектор АКПП (без М режима). 4 состояния уже определены, их трогать нельзя. Если машина заведена, то горит одна из лампочек, а заглушена, то все лампочки не горят. И вот очень чешутся руки SOLID не нарушить. Первый способ, как это можно реализовать:

using UnityEngine;

namespace Core.Car
{
    public class TransmissionSelector : MonoBehaviour
    {
        [SerializeField] private LightFixture _p;
        [SerializeField] private LightFixture _r;
        [SerializeField] private LightFixture _n;
        [SerializeField] private LightFixture _d;

        public bool Enabled { get; set; }
        public TransmissionMode TransmissionMode { get; set; }

        private void Update()
        {
            _p.SetLight(TransmissionMode == TransmissionMode.PARKING && Enabled);
            _r.SetLight(TransmissionMode == TransmissionMode.REVERSE && Enabled);
            _n.SetLight(TransmissionMode == TransmissionMode.NEUTRAL && Enabled);
            _d.SetLight(TransmissionMode == TransmissionMode.DRIVING && Enabled);
        }
    }
}

Но данный код очень... прямолинейный. Хардкод. Никакой расширяемости. Пришла в голову другая идея, сделать два скрипта, первый - это универсальный селектор, а второй - лампочка, которая помнит состояние, на которое она реагирует и состояние включенности.

Класс селектора:

using UnityEngine;

namespace Core.Car
{
    public class TransmissionSelector : MonoBehaviour
    {
        [SerializeField] private TransmissionSelectorLamp[] _lamps;

        public bool Enabled { get; set; }
        public TransmissionMode TransmissionMode { get; set; }

        private void Update()
        {
            foreach (var lamp in _lamps)
            {
                lamp.Enabled = Enabled;
                lamp.CurrentMode = TransmissionMode;
            }
        }
    }
}

Класс лампочки:

using UnityEngine;

namespace Core.Car
{
    [RequireComponent(typeof(LightFixture))]
    public class TransmissionSelectorLamp : MonoBehaviour
    {
        [SerializeField] private TransmissionMode _mode;

        private LightFixture _lamp;

        public TransmissionMode CurrentMode { private get; set; }
        public bool Enabled { private get; set; }

        private void Awake()
        {
            _lamp = GetComponent<LightFixture>();
        }

        private void Update()
        {
            _lamp.SetLight(_mode == CurrentMode && Enabled);
        }
    }
}

И вот в чем проблема. Лампочка теперь знает, на какой сигнал она загорается! Это ужасное архитектурное решение. Зато расширяемое.

Оба эти решения больны тем, что имеется данная запись:

TransmissionMode == SomeMode && Enabled

Здесь уже нарушение человеческой логики! В одном логическом выражении состояние включенности и сравнение состояния режима трансмиссии. Это как говорить, что "Я подпрыгну, если у тебя красное яблоко и свет включен". Нерационально.

Вопрос. Как быть? Какой вариант оставить? Или есть нормальное решение данного вопроса?

Ответы

▲ 1Принят

Проблема почему это не SOLID не в TransmissionMode == SomeMode && Enabled, это полная фигня.

  • нарушен фундаментальный принцип, Single Responsobility! Класс TransmissionSelector имеет ответственность состояния и визуализации лампочек, почему трансмиссия вообще в курсе о существовании какого-то там блока лампочек на приборной панели? Как в коде, так и в реальной жизни.
  • Update обновляющий лампочки каждый кадр что бы что? 99% этих обновлений ничего не обновят, код просто толчит говно в ступе.
  • у трансмиссии есть нейтралка, а Enabled это ваще чё? Не к лампочка ли это относится? Из-за нарушенной ответственности не ясно... Но даже если это так, то это скорее состояние электропитания авто, а не лично лампочек, что опять нарушает ответственность и вообще саму логику происходящего.

Одина ответственность - один класс:

  • есть Transmission, которая по команде (методу, а не свойству) меняет состояние, свойство может быть { get; private set } ({ private get; set; } быть не может от слова СОВСЕМ), не плохо было бы иметь event смены состояния! Класс по сути поле класса Car, MonoBehaviour ему быть не к чему.
  • есть панель с лампочками, некий TransmissionIndicator и это чисто визуализация, соответственно модель и бизнесс логика о её существовании знать не можна. Все что он должен делать, подписаться на событие трансмиссии и менять состояние лампочек по факту изменения, а не долбиться каждый кадр.
  • не SOLID все едино, нормальному неймингу посвящают не меньше времени. То что можно было бы назвать TransmissionSelector это ручка коробки передач, которая в игре тоже является исключительно визуализацией и не более, поскольку в архитектура кода, в отличие от реальности мы меняем передачу напрямую.
▲ -3
  1. Зачем стороннее ПО LightFixture? В юнити сделан свой свет. Просто light :/
  2. [SerializedField] private можно просто сократить до public
  3. В си # в классе все по умолчанию private, private void Update - лишнее.
  4. Первый вариант лучше, используйте switch. Его специально разработали для enum.
  5. В юнити параметр enabled есть по умолчанию. Свой зачем добавлять?
  6. Не надо в юнити солид тащить, юнити - это скрипт. Не надо программировать, надо скриптовать. В скриптах не имеет значения архитектура. Вы не делаете вычислительное ПО, где это важно.
    public class Akpp : MonoBehaviour{
        public GameObject p, r, n, d;
        public TransmissionMode TransmissionMode;

        void Update(){
            p.GetComponent<Light>().enabled = false;
            n.GetComponent<Light>().enabled = false;
            d.GetComponent<Light>().enabled = false;
            r.GetComponent<Light>().enabled = false;

            switch(TransmissionMode){
                case TransmissionMode.P:
                    p.GetComponent<Light>().enabled = true;
                    break;
                case TransmissionMode.D:
                    d.GetComponent<Light>().enabled = true;
                    break;
                case TransmissionMode.N:
                    n.GetComponent<Light>().enabled = true;
                    break;
                case TransmissionMode.R:
                    r.GetComponent<Light>().enabled = true;
                    break;
            }
        }
    }