Comments 9
Спасибо за статью. Небольшой Cod Review, если Вы не против.
Зачем понадобилось усложнять и делать родительский класс, если почти наверняка будет всего один тип ракет в игре (SOLID, KISS, YAGNI)?
Класс Rocket не должен знать про кнопку старта (он должен быть как можно тупее — помните KISS?). Вообще, Rocket перегружен и нарушает множество принципов, перечисленных в первой статье. Код ракеты должен только помогать отображать эту ракету и помогать изменять её положение на экране. При этом это должны быть несколько несвязных между собой компонентов на gameObject ракеты (и/или его частях). Ваш класс Rocket больше похож на RocketMovement. При этом может быть ещё RocketFire, RocketHealth, RocketFuel и т.д.
Код под каждым case должен быть вынесен в отдельный метод. Но, вообще, класс Rocket ничего не должен знать про inputController. У Rocket должен быть отдельный компонент Driver или Engine, который должен иметь что-то типа SetTrust(float trust);
Зачем нужен агрегатор событий ещё и видимый всеми? Почему не использовать обычный static Action someAction в нужных классах и подписки на него? Я видел Вашу статью про этот агрегатор но так и не увидел достоинств усложнения и замедления кода.
Неверное именование. События называют просто Change/Click/Update а методы подписчиков уже OnChange, OnClick, OnUpdate.
Вообще, начинающие разработчики Unity (я и сам когда-то был в их числе) долгое время не понимают идеологии Unity. Идеология такая — если это что-то визуальное (звуковое и т.д.) на сцене, то это gameObject (или entity) на котором нужные компоненты, которые помогают его «визуализировать» и ничего больше. Всё, что не относится к визуализации, лучше выносить отдельно. Т.е. это чем-то похоже на MVC (gameObject это View) но совсем не MVC.
Если посмотреть на MonoBehaviour то видно, что внутри всё помогает отображать, анимировать, двигать объект и так далее. Логика должна быть снаружи.
Т.е. класс Rocket можно упразднить и разбить его на несколько компонентов и управлять этими компонентами уже снаружи не позволяя им знать детали реализации. Т.е. очень просто говоря — ракета, это очередной визуальный компонент, такой как button или text и, конечно, она ничего не должна знать про другие кнопки (startButton) и прочие inputController, события, глобальные состояния игры и прочее. Также, Rocket gameObject должен запросто допускать добавления новых компонентов на себя (SOLID) и, по хорошему, ещё и удаление любых компонентов. Когда проектируете Rocket задумайтесь — можно ли будет изменить её поведение просто добавив или убрав компоненты из gameObject. В случае Вашего прототипа это невозможно — события и прочее глубоко зашиты в класс.
public class Rocket : PlayerRocketBaseЗачем понадобилось усложнять и делать родительский класс, если почти наверняка будет всего один тип ракет в игре (SOLID, KISS, YAGNI)?
protected override void StartEventReact(ButtonStartPressed buttonStartPressed)Класс Rocket не должен знать про кнопку старта (он должен быть как можно тупее — помните KISS?). Вообще, Rocket перегружен и нарушает множество принципов, перечисленных в первой статье. Код ракеты должен только помогать отображать эту ракету и помогать изменять её положение на экране. При этом это должны быть несколько несвязных между собой компонентов на gameObject ракеты (и/или его частях). Ваш класс Rocket больше похож на RocketMovement. При этом может быть ещё RocketFire, RocketHealth, RocketFuel и т.д.
switch (rocketState)
{
case RocketState.WAITFORSTART:
if (inputController.OnTouch && !inputController.OnDrag)
rocketHolder.RotateHolder(inputController.worldMousePos);
break;Код под каждым case должен быть вынесен в отдельный метод. Но, вообще, класс Rocket ничего не должен знать про inputController. У Rocket должен быть отдельный компонент Driver или Engine, который должен иметь что-то типа SetTrust(float trust);
GlobalEventAggregator.EventAggregator.Invoke(new ButtonStartPressed());Зачем нужен агрегатор событий ещё и видимый всеми? Почему не использовать обычный static Action someAction в нужных классах и подписки на него? Я видел Вашу статью про этот агрегатор но так и не увидел достоинств усложнения и замедления кода.
public Action<T> OnChange;Неверное именование. События называют просто Change/Click/Update а методы подписчиков уже OnChange, OnClick, OnUpdate.
Вообще, начинающие разработчики Unity (я и сам когда-то был в их числе) долгое время не понимают идеологии Unity. Идеология такая — если это что-то визуальное (звуковое и т.д.) на сцене, то это gameObject (или entity) на котором нужные компоненты, которые помогают его «визуализировать» и ничего больше. Всё, что не относится к визуализации, лучше выносить отдельно. Т.е. это чем-то похоже на MVC (gameObject это View) но совсем не MVC.
Если посмотреть на MonoBehaviour то видно, что внутри всё помогает отображать, анимировать, двигать объект и так далее. Логика должна быть снаружи.
Т.е. класс Rocket можно упразднить и разбить его на несколько компонентов и управлять этими компонентами уже снаружи не позволяя им знать детали реализации. Т.е. очень просто говоря — ракета, это очередной визуальный компонент, такой как button или text и, конечно, она ничего не должна знать про другие кнопки (startButton) и прочие inputController, события, глобальные состояния игры и прочее. Также, Rocket gameObject должен запросто допускать добавления новых компонентов на себя (SOLID) и, по хорошему, ещё и удаление любых компонентов. Когда проектируете Rocket задумайтесь — можно ли будет изменить её поведение просто добавив или убрав компоненты из gameObject. В случае Вашего прототипа это невозможно — события и прочее глубоко зашиты в класс.
Спасибо за ответ и код ревью ) Особенно за начинающего программиста. Долго отвечал — был в кино, советую кстати Джонни Уика.
Итак:
1)
2)
3)
4)
5)
6)
7)
COOP люблю, хорошая штука, но надо взглянуть на вещи шире — компонент Unity, это по факту куча барахла который тащит с собой монобех. Выделять функционал в отдельный юнити компонент, лучше когда он действительно отдельный и прям либо ваще никак не связан с конкретным классами. Например компонент для дефолтной озвучки UI(клик, наведение и тд), он вообще может ни про кого не знать и действовать автономно. В случае с ракетой — у неё только поведение связанное с движением. Допустим у нас всё становится сложнее — появляется стрельба, защитные поля, хелс поинты и тд, и допустим появятся враги, у которых будет такой же функционал (чисто фантазии). То этот функционал лучше разнести по классам моделям. И будет модель движения, модель стрельбы и тд. Опять же если смотреть шире — класс модель, и есть компонент в каком то смысле, ничто не мешает внедрить этот класс любому заинтересованному лицу. Пример в этом коде — ForceModel forceModel, этот класс можно применить например к астериодам, и они тоже станут попадать под влияние модификаторов. Плюс не забываем про любимый ООП, и у наследников мы можем переопределить или расширить функционал. Я против изначального дробления на мифические компоненты, вот это как раз и есть нарушения KISS в чистом виде. Вместо простого класса с простым поведением, мы пытаемся заложиться в COOP, плодим классы, плодим монобехи, каждый крутит свой апдейт, складирует в памяти кучу гавна которое не используется и тд. Те же Unity поняли что COOP в Unity тупиковый для оптимизации, и поэтому активно пилят ECS для слабых мест, который как раз выигрывает в том числе за счёт того что не тащит с собой целый багаж ненужных штук, которые валяются в куче.
Итак:
1)
Зачем понадобилось усложнять и делать родительский класс, если почти наверняка будет всего один тип ракет в игреМы рассматривали возможность нескольких вариантов ракет, одна из которых может смещаться в полёте.
2)
Класс Rocket не должен знать про кнопку стартаНу он и не знает, ракета только знает что ей прилетает ивент на старт
3)
Код под каждым case должен быть вынесен в отдельный метод.Согласен, я просто придерживаюсь правила — если кода не очень много, можно оставить в кейсе. Но в отдельный метод будет правильнее.
4)
Но, вообще, класс Rocket ничего не должен знать про inputController. У Rocket должен быть отдельный компонент Driver или Engine, который должен иметь что-то типа SetTrust(float trust);В любом деле главное без фанатизма, инпут контроллер в данном ракурсе не верхняя сущность, а всего лишь источник данных для принятия решений, да, в нём есть лишние данные для ракеты, более расово правильно это было всё связать через интерфейс с конкретными штуками, но снаружи мы не можем менять данные, поэтому если торчат пара лишних для ракеты полей — ничего страшного. Всё равно эти данные будут необходимы, если не ракете, так более верхней сущности/объекту.
5)
Зачем нужен агрегатор событий ещё и видимый всеми?Я на самом деле понимаю что у меня есть проблема с позиционированием агрегатора, ибо под капотом там ивенты, механика работы у него — по сути потоки данных, а основная фича — именно контейнеры данных. Видимость всеми — ну в этом и фича, подписка на нужный тебе тип данных. В третьей части будут модификаторы которые просто присылают данные о модификаторе, может будет более очевидно, не говоря уже о инъекциях.
6)
Неверное именование. События называют просто Change/Click/Update а методы подписчиков уже OnChange, OnClick, OnUpdate.склонен согласиться.
7)
Вообще, начинающие разработчики Unity (я и сам когда-то был в их числе) долгое время не понимают идеологии Unity. Идеология такая — если это что-то визуальное (звуковое и т.д.) на сцене, то это gameObject (или entity) на котором нужные компоненты, которые помогают его «визуализировать» и ничего больше. Всё, что не относится к визуализации, лучше выносить отдельно. Т.е. это чем-то похоже на MVC (gameObject это View) но совсем не MVC.
Если посмотреть на MonoBehaviour то видно, что внутри всё помогает отображать, анимировать, двигать объект и так далее. Логика должна быть снаружи.
COOP люблю, хорошая штука, но надо взглянуть на вещи шире — компонент Unity, это по факту куча барахла который тащит с собой монобех. Выделять функционал в отдельный юнити компонент, лучше когда он действительно отдельный и прям либо ваще никак не связан с конкретным классами. Например компонент для дефолтной озвучки UI(клик, наведение и тд), он вообще может ни про кого не знать и действовать автономно. В случае с ракетой — у неё только поведение связанное с движением. Допустим у нас всё становится сложнее — появляется стрельба, защитные поля, хелс поинты и тд, и допустим появятся враги, у которых будет такой же функционал (чисто фантазии). То этот функционал лучше разнести по классам моделям. И будет модель движения, модель стрельбы и тд. Опять же если смотреть шире — класс модель, и есть компонент в каком то смысле, ничто не мешает внедрить этот класс любому заинтересованному лицу. Пример в этом коде — ForceModel forceModel, этот класс можно применить например к астериодам, и они тоже станут попадать под влияние модификаторов. Плюс не забываем про любимый ООП, и у наследников мы можем переопределить или расширить функционал. Я против изначального дробления на мифические компоненты, вот это как раз и есть нарушения KISS в чистом виде. Вместо простого класса с простым поведением, мы пытаемся заложиться в COOP, плодим классы, плодим монобехи, каждый крутит свой апдейт, складирует в памяти кучу гавна которое не используется и тд. Те же Unity поняли что COOP в Unity тупиковый для оптимизации, и поэтому активно пилят ECS для слабых мест, который как раз выигрывает в том числе за счёт того что не тащит с собой целый багаж ненужных штук, которые валяются в куче.
Особенно за начинающего программиста
Я писал не про начинающего программиста, а про начинающего разработчика на Unity. Я до сих пор время от времени сам себя считаю таковым.
Мы рассматривали возможность нескольких вариантов ракет
Тут, как раз, и пригодится это дробление на компоненты, а не базовый общий класс.
Ну он и не знает, ракета только знает что ей прилетает ивент на старт
И про этот event не должна знать, как и какая-нибудь кнопка не должна знать про event StartGame, например. Ракета должна управляться извне (я писал про пример двигателя) или отдельным компонентом (набором компонентов), который можно заменить на другой. И вообще не должна знать что такое event.
что у меня есть проблема с позиционированием агрегатора
На мой взгляд проблема не с позиционированием, а с тем, что это слишком общее решение.
это по факту куча барахла который тащит с собой монобех
Верно.
лучше когда он действительно отдельный и прям либо ваще никак не связан с конкретным классами
Верно! Поэтому нужно писать именно такие компоненты. Чем меньше связность, тем лучше.
Я против изначального дробления на мифические компоненты
Придётся. От DOTS уже никуда не убежишь. Лучше начать сейчас, чем опоздать.
вот это как раз и есть нарушения KISS в чистом виде
Дробление не приводит к усложнению.
и поэтому активно пилят ECS для слабых мест
Это уже по сути случилось. Я понимаю про какую именно оптимизацию Вы пишете. Но всё же это самое «дробление», тем более в Вашем случае того точно стоит.
Только что был по лекции про DOTS, не везде он зайдет, и там очень много писанины, далее:
1) Компоненты у Unity слабый элемент, поэтому я в большинстве случаев за наследование и композицию. Композиция если брать абстрактно и есть парадигма COOP. Просто либо мы пользуемся обертками Unity со всеми вытекающими, либо сами пишем и дробим функционал как нам хочется. И скажем так — текущие компоненты это для удобства инди разработчика, понавесил какой то функционал, запихал в публичные поля объекты, и оно взлетело.
2)
3) Насчёт DOTS/ECS — ну это попытка пропихнуть DDD в более качественной обертке с волшебными штуками, но по факту у неё есть те же слабые места что и у DDD. Другой вопрос что при определенных задачах профит от DOTS очень крут. А вот то что писанины становится в разы больше и она не всегда оправдана — это факт. Порой для описания простой сущности нужны десятки объектов. Поэтому для простого прототипа казуалки я бы избегал нативного ECS.
1) Компоненты у Unity слабый элемент, поэтому я в большинстве случаев за наследование и композицию. Композиция если брать абстрактно и есть парадигма COOP. Просто либо мы пользуемся обертками Unity со всеми вытекающими, либо сами пишем и дробим функционал как нам хочется. И скажем так — текущие компоненты это для удобства инди разработчика, понавесил какой то функционал, запихал в публичные поля объекты, и оно взлетело.
2)
И про этот event не должна знатьНе согласен, вполне себе в рамках событийной модели решение. Как еще ракете узнать что пора? Вешать более верхнюю сущность и хендлить там? Что еще будет делать эта сущность? Зачем она? Откуда ракета в принципе узнает что пора? В данный момент есть условный поток данных — старт ракеты, где может быть весь необходимый контекст. Всем кому интересны эти данные — реагируют.
3) Насчёт DOTS/ECS — ну это попытка пропихнуть DDD в более качественной обертке с волшебными штуками, но по факту у неё есть те же слабые места что и у DDD. Другой вопрос что при определенных задачах профит от DOTS очень крут. А вот то что писанины становится в разы больше и она не всегда оправдана — это факт. Порой для описания простой сущности нужны десятки объектов. Поэтому для простого прототипа казуалки я бы избегал нативного ECS.
Только что был по лекции про DOTS, не везде он зайдет
На самом деле, даже не с точки зрения производительности, а именно с точки зрения архитектуры он зайдёт много где.
и там очень много писанины
Да, и при этом, как это ни странно, cоблюдаются все эти SOLID, KISS etc, в отличии от.
либо мы пользуемся обертками Unity со всеми вытекающими
Unity обязывает ими пользоваться (в DOTS в том числе, но под другим соусом) если нужно представление чего-либо на сцене. Тут важно понимать (и я писал об этом) где нужно использовать MonoBehaviour, а где нет.
Как еще ракете узнать что пора?
Этот вопрос созвучен с вопросом «как узнать кнопке (т.е. по сути такому-же визуальному объекту, что и ракета), что её нужно показать/скрыть?». Решения могут быть разными, но не изменение кода кнопки.
Откуда ракета в принципе узнает что пора?
Она и не должна. Вообще, для ракеты не должно быть понятия «пора». У неё (точнее двигателя) должны быть функции PrepareToStart(), SetTrust(float trust) и т.д. Ракета не должна быть настолько умной, чтобы «знать, что пора».
В данный момент есть условный поток данных — старт ракеты
И это проблема — сложно тестировать поведение (например, двигателей) — нужно писать отдельный функционал для тестов. Другая проблема — inputController который жёстко зашит внутри ракеты. Что если мы хотим управлять ракетой через тесты? Что если через сеть? Что если через обучающий сценарий, что если хотим записать её полёт и повторить потом? Если бы у ракеты было просто SetTrust(float trust) то всех этих проблем не было бы — делай с ней, что хочешь.
И это проблема — сложно тестировать поведение (например, двигателей) — нужно писать отдельный функционал для тестов. Другая проблема — inputController который жёстко зашит внутри ракеты. Что если мы хотим управлять ракетой через тесты? Что если через сеть? Что если через обучающий сценарий, что если хотим записать её полёт и повторить потом? Если бы у ракеты было просто SetTrust(float trust) то всех этих проблем не было бы — делай с ней, что хочешь.
В данном контексте тестирование супер изи, ибо есть конфиг в котором зашит параметр скорость — запихиваем разный конфиг, можно через инъекцию как раз агрегатором. Далее даём отмашку старт, далее присылаем ивенты добавления модификаторов. Тест занял бы от 2х с плюсом строчек строчек. Это легко делается тестовыми методами/классами, и можно проверить самые сложные сценарии типа применения одновременно нескольких модификаторов с разной силой влияния.
На самом деле я думаю уже можно закругляться, я понял вашу точку зрения. Я придерживаюсь другой и признаю большинство парадигм, и на мой взгляд всему своё место, COOP, OOP, AOP, Reactive, Event based, DDD.
И вопрос с подвохом — DDD придумали не сегодня, почему он не занял доминирующую позицию в программировании?
тестирование супер изи, ибо
И далее идёт описание большой цепочка необходимых действий, некоторые из которых потенциально опасны о могут поломать объект — т.е. есть целый контекст для тестирования, который нужно при необходимости передать/объяснить другому члену команды. «супер изи» тестирование это SetTrust(-1) для отдельного компонента и всё — сейчас так не получится — нужна цепочка.
И вопрос с подвохом
Не люблю вопросы с подвохом — обычно, в них есть какой-нибудь подвох :)
DDD придумали не сегодня, почему он не занял доминирующую позицию в программировании?
И TDD не занял. Пожимаю плечами. Я в комментариях к статье на рассуждаю о доминирующих позициях. Просто написал свой небольшой и далеко не полный review. Например, если продолжать, то этот код правильный:
private float ClampAngle(float angle, float from, float to)А вот этот уже нет:
private Vector3 ClampRotationVectorZ (Vector3 rotation)потому, что он должен был быть таким:
private Vector3 ClampRotationVectorZ (Vector3 rotation, float clampAngle)Класс ReactiveValue делает CurrentValue публичным, что неправильно.
public class ReactiveValue<T> where T: structИ так далее.
В целом мне нравятся Ваши статьи и я просто хочу помочь советом. При этом, конечно, ни на чём не настаиваю — это просто советы и, несомненно, я могу ошибаться.
Sign up to leave a comment.
Прототипирование мобильной игры, с чего начать, и как это делать. Часть 2