Войти
Инди-ЮнитиФорум

Мастер класс по хорошему коду в юнити (2 стр)

Advanced: Тема повышенной сложности или важная.

Страницы: 1 2 3 415 Следующая »
#15
(Правка: 10:48) 10:45, 19 сен. 2020

jkenmor
> Он звучит так: Сколько стоит час Вашего ревью?
в качестве жеста доброй воли и поддерживая затею начатой Саламандром тут, могу посмотреть бесплатно. Лучше конечно за "работу по графике" для моего проекта.

Ну, а так вряд ли вы потяните. От 50 EUR за день работы, часами тут не мерится. Но во время ревью я смотрю все в комплексе, до каждой строчки не докапываюсь, как это делают те кто это делает на показ, выявляю только тяжелые проблемы архитектуры. Что называется в несколько проходов, после выполнения рефакторинга можно пойти на второй круг.

#16
10:51, 19 сен. 2020

BooTheJudge
> покажи уже, что пишут местные дилетанты, мне интересно.
Попробую, но это сложно т.к. они вроде как не давали разрешения публиковать их гавнокод, а он к тому же завязан на использование чужих либ. Делать какую то выжимку - можно не понять идею, но будет время попробую.

#17
16:25, 19 сен. 2020

tac
Ну, а так вряд ли вы потяните.
На завтраках поэкономим, накопим, ничего страшного. Вопрос ещё один. Это Ваша регулярная зарплата или просто оценили свой труд в среднем по рынку?

#18
16:28, 19 сен. 2020

jkenmor
это меньше моей регулярной зарплаты, откуда такой интерес :)

#19
(Правка: 22:26) 21:17, 19 сен. 2020

forwhile
> кастую Alprog'a в топик
а я то уже было и забыл, где я первый раз возмущался про сереализацию ... разговаривали мы с ним, увы, и он подвержен этой болезни :)

А вот как давно это было

А после того, как у Alprog закончились аргументы, поднял отдельную тему

Ладно, там с Alprog мы зашли в дебри о надобности сереализации в принципе (хотя вот его кода я не видел, но прям чую какой там оверхед), но есть примеры и по проще ... скоро дам пример ...

#20
(Правка: 21 сен. 2020, 7:42) 23:22, 19 сен. 2020

Пока кусок кода, который стоит разобрать.

попробуем разобраться, что человек хочет/делает. Задача состояла сделать инструмент, чтобы прямо в редакторе юнити можно было создавать план комнат (а вообще и всего другого, диалоги и прочие, для чего нужна информация в виде графа). Решили использовать NodeGraphProcessor.

В коде выдернут лишь метод Save, который вызывается в окне редактора по отдельной кнопке.

Пока посмотрите, может чего поймете из кода, если нет завтра до расскажу ... но сразу скажу, выпрямить такой говнокод, не один день моей оплаты понадобится :) тут намешено ну прям все.

Как минимум один фейл вы должны видеть ну прям сразу, но вот посмотрим ...

+ Показать

+ Показать


И еще заготовка на будущие код передвижения по комнатам. А тут код говорит сам за себя ... ваши предположения ,что не так?

+ Показать
#21
11:19, 20 сен. 2020

tac
Да интересно, кто чем живёт. Кто работает в геймдеве серьёзно и доволен этим. Кто сколько получает и при каких навыках. На этом форуме всякого можно услышать, сам знаешь)

#22
(Правка: 22:23) 22:21, 20 сен. 2020

Ну, где вы? тут нет программистов? ну позовите их бездари, недоучки, кто не может взять на себя ответственность прокомментировать .. .меня подмывает прокомментиовать эти фейс палмы в коде выше, но вы похоже готовы только троллить ..  а где те, кто отвечают за свои слова? вот он - код, вы ревизоры, не видите ничего?

От себя, ну это же просто дичь, у человека кто написал такой код, что в голове? Я готов объяснять, лишь бы такого не было у коллег, но не тем кто с задранной головой такое пишет .. это господа - синдром недоучки, я добавлю в свою классификацию проблем

#23
23:53, 20 сен. 2020

tac
> От себя, ну это же просто дичь,
вне контекста не понятно, кроме того что куча какого то кода.и видно все можно было сделать парой строчек, но почему то не сделано. Вообще сишарпистам надо было идти романы писать.

#24
(Правка: 26 окт. 2020, 21:26) 7:41, 21 сен. 2020

forwhile
> видно все можно было сделать парой строчек, но почему то не сделано
именно так, но вот почему так не сделано, об этом и пойдет наш рассказ :)

Начнем с конца: код передвижения по комнатам

Там очевидно просматривается паттерн MVC (реализованный криво и не правильно, но то ладно, я действительно еще не видел того, кто это умеет правильно делать). Ведь так? Но зачем он здесь? было бы даже лучше, чтобы те кто занимаются геймдевом об этом паттерне ничего не знали. Я могу утверждать, что он им не понадобится от слова совсем. А тут видимо парень начитался и давай его сувать куда попало ... вопрос: зачем же применяют этот паттерн и когда он нужен? (после ответа на это продолжу разбор)

Заодно обратите внимание на

async void Move(Direction direction)

твою ж ... ему было мало применения MVC с доморощенными событиями, так он еще решил добавить ассинхроности, чтобы потом героически

await Task.CompletedTask;

ждать синхронизации. Причем и сверху метод синхронный

private void SignalMoveDirectionCallback(SignalMoveDirection signalMoveDirection)
    {
        Move(signalMoveDirection.Direction);
    }

вот что у человека в голове, он хочет показать все что он умеет? или сразу использовать все фичи языка :)

теперь то, что видно из кода выше, подсказка, обратите внимание на

            Location.Rooms.Clear();
            foreach (var node in nodes)
            {
                var roomGraphNode = (node as RoomGraphNode);
                roomGraphNode.Room.MapPosition = CalcMapPosition(roomGraphNode);
                roomsPositionsDict.Add(GetRoomPositionString(roomGraphNode.Room.MapPosition),  roomGraphNode.Room);
                roomGraphNode.Room.Location = Location;
                EditorUtility.SetDirty(roomGraphNode.Room);
                Location.Rooms.Add(roomGraphNode.Room.MapPosition, roomGraphNode.Room);
            }

зачем два словаря? (да еще по разному, с привлечением доп. методов, делающий одно и тоже) один глобальный и еще локальный?

и замете, это ни супер пупер какая игра, это текстовая игра аля Dwarf Fortress

а то, что возможно плохо видно из кода - так это то, что этот метод Save вообще не нужен. Он делает инструмент для создания графов. При этом комнаты у него линиями не соединены, а он вычисляет их относительное положение из расположения в окне (справа, слева, сверху, снизу). Это почему? Так и не смог научится использовать библиотеку, чтобы связывать ноды графа?

Я уже не говорю о том, что если убрать визуализацию графа из проекта (библиотеку NodeGraphProcessor), т.к. в сборке игры она не нужна, то все приехали класс Location  не сможет пересчитать, кто справа, кто слева?

Дальше, он еще заставил отдельно жать кнопку сохранения. А сделать то, что тебе нужно во время создания нодов и линков никак нельзя? Причем что же ему нужно? Создать отдельный объект Location и каждый раз при Save его заново пересоздать и сериализовать. Причем не стандартными методами Юнити, а привлекая odin лишь для того, чтобы сериализовать словарь, а без словаря никак обойтись нельзя (может подарить вам класс, позволяющий работать через словарь, но с сериализацией в два списка, как то даже советуют в документации Юнити)? Надо сраную библиотеку сериализации привлекать?

Ну и наконец, вместо того, чтобы обобщить все это .. выделив где у тебя граф, а где особенности комнат/диалогов/точек сценария и т.п. что может представляться графом, он каждый раз пишет такую же хрень и для других вещей ... городя огород ...

Так вот я спрашиваю, ну как же можно сотрудничить с такими недоучками, которые к тому же еще мнят о себе .. им бы поучится вначале, а не команды собирать, или сидеть тихо и слушать как надо программировать ..
(обидно наверно написал, но просто своим ЧСВ автор этого кода меня завел ... так то, я понимаю все учатся, я тогда могу и спокойно пояснять)

#25
(Правка: 8:39) 8:27, 21 сен. 2020

чет я вошел во вкус :) Ну, что кто даст код на ревью :) ?

лан, пойду лучше делать свою игру, лучше медленно, но зато надежно
#26
12:19, 21 сен. 2020

tac
Ну ты напросился) много кода тебе нужно, чтоб познать всю глубину наших глубин?)

#27
16:51, 21 сен. 2020

jkenmor
что не жалко показать :)

#28
17:43, 21 сен. 2020

/// <summary>
    /// Класс, описывающий куб.
    /// </summary>
    public class Dice : IRollable
    {
        /// <summary>
        /// Находится ли куб в пуле(карта активирована)
        /// </summary>
        public bool IsActivated { get; set; }
        /// <summary>
        /// Выбран ли куб в течении хода.
        /// </summary>
        public bool IsSelected { get; set; }

        /// <summary>
        /// Сторона куба.
        /// </summary>
        public Symbol RolledSymbol { get; set; }
        /// <summary>
        /// Символы на кубе.
        /// </summary>
        public IEnumerable<Symbol> Symbols { get; }

        /// <summary>
        /// Стандартный конструктор.
        /// </summary>
        ///
        public Dice(IEnumerable<Symbol> symbols)
        {
            Symbols = symbols;
        }

        /// <summary>
        /// Активирует куб.
        /// </summary>
        public void Activate()
        {
            IsActivated = true;
        }

        /// <summary>
        /// Применяет куб.
        /// </summary>
        public void Resolve()
        {
            IsActivated = false;
        }

        public void GetSide(Transform transform)
        {
            var dicePosition = transform.localEulerAngles;

            if (Round(dicePosition.x) && Round(dicePosition.z))
            {
                RolledSymbol = Symbols.ToArray()[0];
                return;
            }
            if (Round(dicePosition.x) && Math.Abs(dicePosition.z - 180) < 0.1f)
            {
                RolledSymbol = Symbols.ToArray()[1];
                return;
            }
            if (Turn(dicePosition.x) && Round(dicePosition.z))
            {
                RolledSymbol = Symbols.ToArray()[2];
                return;
            }
            if (Round(dicePosition.x) && Turn(dicePosition.z))
            {
                RolledSymbol = Symbols.ToArray()[3];
                return;
            }
            if (Round(dicePosition.x) && Math.Abs(dicePosition.z - 180) < 0.1f)
            {
                RolledSymbol = Symbols.ToArray()[4];
                return;
            }
            if (Round(dicePosition.x) && Math.Abs(dicePosition.z - 180) < 0.1f)
            {
                RolledSymbol = Symbols.ToArray()[5];
                return;
            }
        }

        private bool Round(float angle)
        {
            return Math.Abs(angle) < 0.1f || Math.Abs(angle - 360) < 0.1f;
        }
        private bool Turn(float angle)
        {
            return Math.Abs(angle - 90) < 0.1f || Math.Abs(angle - 270) < 0.1f;
        }
    }

Такой вот говнокод. Задача: шестигранный куб. Значения на кубе не 1, 2, 3 и т.д., разные символы, разные значения. Значения задаются отдельно. Куб является физическим объектом и падает на стол. Нужно понять, какой стороной вверх повёрнут куб.

#29
(Правка: 18:31) 18:17, 21 сен. 2020

jkenmor
ну это очень мелко для меня ... так то я смотрю целые проекты ... ну хорошо, тут пусть будем "в малом"

1. IRollable

Использование интерфейсов меня всегда настораживает, для чего он здесь, тут есть некие сущности, которые тоже крутятся и нет возможности сделать наследование?

2. IEnumerable
Серьезно, это нужно? Почему не просто List ?

3. var
это чисто стиль, но настойчиво рекомендую не пользоваться этим ключевым словом, не ленитесь и пишите конкретный тип, это очень плохо читается, надо всегда думать какого он типа. А у нас язык строго типизированный.

Дальше по сути

4. GetSide
4.1. там стоило бы применить цикл, а не писать кучу if (альтернатива switch, если лень обобщать )
4.2. нельзя же так - использовать return надо в одном месте, в конце, есть такое правило одна точка выхода (исключение проверка входных параметров в самом начале, и выброс ошибки)
4.3. Чтобы сделать цикл нужно обобщить условие, оно должно быть единым. Там видимо где-то еще опечатка, последние два условия повторяются. А чтобы обобщить статичную информацию об угле и что там еще надо, стоит записать в класс Symbol

условие будет выглядеть вида
if ( (Math.Abs(dicePosition.x + Symbols_i.AddX1) < 0.1f || Math.Abs(dicePosition.x  + Symbols_i.AddX2) < 0.1f) &&  Math.Abs(dicePosition.z + Symbols_i.AddZ1) < 0.1f || Math.Abs(dicePosition.z  + Symbols_i.AddZ2) < 0.1f)

для пущей чистоты 0.1 - завести настроечный параметр

4.4. Избавьтесь от методов Round/Turn
4.5. GetSide(Transform transform) - как параметр лучше использовать Vector3 собственно углы

Страницы: 1 2 3 415 Следующая »
Инди-ЮнитиФорум