1 Тема від FakiNyan Востаннє редагувалося FakiNyan (21.08.2016 20:50:29)

  • FakiNyan
  • FakiNyan
  • Робітниця.
  • Поза форумом
  • Дата реєстрації: 27.06.2013
  • Повідомлень: 14 116
    • Репутація: 5674

Тема: Проаналізуйте код мого проекту, будь-ласочка

Прів. Суть в тому, що я повний нуль в архітектурі, а хочу стати крутим пограмістом з нормальним кодом, котрий і логічно виглядає, і легко читається, і легко змінюється.

Озьдо посилання на проект
https://github.com/ChazAshley/secret-game2
всі скрипти в папці Assets/scripts

Невеличке пояснення по скриптам.

GroundController контролює текстурку, котра виступає в якості землі. В цьому скрипті змінюється офсет текстурки, аби виглядало так, наче гравець рухається вперед (текстура зсувається вниз).
Також в цьому скрипті умовна земля розбивається на ділянки, кожна з котрих має таку ширину, аби на ній поміщалась текстурка з чоловічком. Ще цей скрипт керує додаванням чоловічків на землю.

HumanController контролює чоловічків, котрі додаються в класі GroundController. Чоловічки використовуєть швидкість руху землі, аби порахувати свою швидкість, котра може бути від 20% до 100% швидкості землі. Коли чоловічок заходить за нижній край екрану, то зникає, і відправляє повідомлення про це класу GroundController, аби той створив нового чоловічка.

MovementController управляє позицією пташки, котра виступає персонажом гри. Ця пташка може какулькати, якщо натиснете на space, тому я вирішив додати приціл, котрий змінює свою позицію відповідно до руху землі та пташки, цей приціл показує місце на землі, де впаде какулька.

PoopController управляє какулькою. При додаванні какульки в сцену, вона рухається від дупи пташки (місце, в котрому створюється какулька), до місця, куди вказував приціл в момент створення какульки. При цьому вона зменшується в 2 рази за час польоту. Коли вона падає, то перевіряється, чи є під нею людина, якщо людина є, то далі какулька рухається зі швидкістю цієї людини, якщо немає, то зі швидкістю землі.


Мені тре коментарі, як зробити цей дурний код таким, аби круті пограмісти з крутих контор могли прийняти його за свій, звісно, якби їм дали завдання - написату ось таку ігрульку.

ось посилання на збірку для віндуса
https://www.dropbox.com/s/wflc7qh0c1z7w … e.zip?dl=1

ось для лінуха
https://www.dropbox.com/s/4x3ct9r4g3591 … x.zip?dl=1

2 Відповідь від Master_Sergius

  • Дата реєстрації: 01.10.2014
  • Повідомлень: 1 068
    • Репутація: 805

Re: Проаналізуйте код мого проекту, будь-ласочка

1. Як то все зібрати і запустити повинно міститися в інструкції прямо на GitHub, зазвичай це все той же README
2. Оці ось "невеличкі пояснення по скриптах" повинні бути в самих скриптах. Взагалі, варто писати якісь коментарі з поясненнням окремих шматків коду, а в ідеалі - до кожного методу.
3. Закоментовані шматочки коду типу "може колись згодиться" якраз то не повинно бути. Та й система контролю версій дозволяє повключати ті шматки в якісь комміти, найкраще в якусь іншу гілку.
4. "Щоб круті програмісти з крутих контор могли прийняти за свій" - хоч на одну співбесіду ходили вже? Навіть провальна співбесіда може бути корисна в тому сенсі, що будете орієнтуватися що зара "в тренді" і чого Вам не вистачає. А то так какульки і лишаться.

Подякували: 221VOLT, FakiNyan, leofun013

3 Відповідь від quez

  • quez
  • quez
  • Replace Team
  • Поза форумом
  • Звідки: Новий Єрусалим
  • Дата реєстрації: 01.05.2014
  • Повідомлень: 3 103
    • Репутація: 1621

Re: Проаналізуйте код мого проекту, будь-ласочка

Input.GetKeyDown(KeyCode.D)

Якщо робите код, щоб показувати іншим, то клавіші не варто хардкодити.

Під лінуксом шось не запускається білд, there is no data folder

Подякували: FakiNyan, leofun012

4 Відповідь від FakiNyan

  • FakiNyan
  • FakiNyan
  • Робітниця.
  • Поза форумом
  • Дата реєстрації: 27.06.2013
  • Повідомлень: 14 116
    • Репутація: 5674

Re: Проаналізуйте код мого проекту, будь-ласочка

quez написав:
Input.GetKeyDown(KeyCode.D)

Якщо робите код, щоб показувати іншим, то клавіші не варто хардкодити.

Під лінуксом шось не запускається білд, there is no data folder

похвіксив, наче, мона перезавантажити

5 Відповідь від quez

  • quez
  • quez
  • Replace Team
  • Поза форумом
  • Звідки: Новий Єрусалим
  • Дата реєстрації: 01.05.2014
  • Повідомлень: 3 103
    • Репутація: 1621

Re: Проаналізуйте код мого проекту, будь-ласочка

 var nhTrans = newHuman.transform as RectTransform;

Нащо це, якщо далі ви все одно використовуєте тільки успадковані властивості і методи?

Подякували: leofun011

6 Відповідь від FakiNyan

  • FakiNyan
  • FakiNyan
  • Робітниця.
  • Поза форумом
  • Дата реєстрації: 27.06.2013
  • Повідомлень: 14 116
    • Репутація: 5674

Re: Проаналізуйте код мого проекту, будь-ласочка

quez написав:
 var nhTrans = newHuman.transform as RectTransform;

Нащо це, якщо далі ви все одно використовуєте тільки успадковані властивості і методи?

колись розробники юніті казали, шо якщо зробити змінну, в котру загнати transform, і звертатись вже до неї, а не до transform, то воно буде швидше працювати

7 Відповідь від quez

  • quez
  • quez
  • Replace Team
  • Поза форумом
  • Звідки: Новий Єрусалим
  • Дата реєстрації: 01.05.2014
  • Повідомлень: 3 103
    • Репутація: 1621

Re: Проаналізуйте код мого проекту, будь-ласочка

Я про as

Подякували: FakiNyan1

8 Відповідь від FakiNyan Востаннє редагувалося FakiNyan (21.08.2016 21:50:48)

  • FakiNyan
  • FakiNyan
  • Робітниця.
  • Поза форумом
  • Дата реєстрації: 27.06.2013
  • Повідомлень: 14 116
    • Репутація: 5674

Re: Проаналізуйте код мого проекту, будь-ласочка

quez написав:

Я про as

а, тоді хз
майбуть, думав спочатку, шо буду юзати фічі RectTransform'у, а потім бачте як воно вийшло..

9 Відповідь від iovchynnikov Востаннє редагувалося iovchynnikov (22.08.2016 08:08:08)

  • Звідки: Спочивальня Бандери
  • Дата реєстрації: 24.01.2016
  • Повідомлень: 496
    • Репутація: 589

Re: Проаналізуйте код мого проекту, будь-ласочка

Чесно кажучи, виглядає як мішок функціонального коду обгорнутий у методи класів.
Почніть з моделювання доменних об'єктів: людини, пташки. Бо це якийсь нонсенс:

var hc = newHuman.GetComponent<HumanController>();

Я б тут очікував:

val human = new Human(args);
//abo
val human = HumanFactory.build(arg);

А потім працювати з цим Human об'єктом, який був би зв'язаний з юніті, наприклад, патерном Observer. Вийшло б досить low coupled.
Саме у Human класі має бути уся логіка, пов'язана з ним (DDD). Не робіть товстих контролерів.
Розділення на контроллери теж дивне. Варто їх розрізняти не за функціями, а за сутностями і такими, що відповідають вимозі Single responsibility principle (SOLID): GroundCtrl не має відповідати за людей (делегуйте це HumanCtrl), a MovementCtrl має увійти до BirdCtrl, котрий буде містити все що стосується птахів.

PS: абсолютно не знаюся на шарпі і юніті. Поради суто архітектурні. Можливо деякі, а може й всі, будуть не будуть пасувати до Вашого прикладу.

Подякували: FakiNyan, leofun012

10 Відповідь від FakiNyan

  • FakiNyan
  • FakiNyan
  • Робітниця.
  • Поза форумом
  • Дата реєстрації: 27.06.2013
  • Повідомлень: 14 116
    • Репутація: 5674

Re: Проаналізуйте код мого проекту, будь-ласочка

iovchynnikov написав:

Чесно кажучи, виглядає як мішок функціонального коду обгорнутий у методи класів.
Почніть з моделювання доменних об'єктів: людини, пташки. Бо це якийсь нонсенс:

var hc = newHuman.GetComponent<HumanController>();

Я б тут очікував:

val human = new Human(args);
//abo
val human = HumanFactory.build(arg);

А потім працювати з цим Human об'єктом, який був би зв'язаний з юніті, наприклад, патерном Observer. Вийшло б досить low coupled.
Саме у Human класі має бути уся логіка, пов'язана з ним (DDD). Не робіть товстих контролерів.
Розділення на контроллери теж дивне. Варто їх розрізняти не за функціями, а за сутностями і такими, що відповідають вимозі Single responsibility principle (SOLID): GroundCtrl не має відповідати за людей (делегуйте це HumanCtrl), a MovementCtrl має увійти до BirdCtrl, котрий буде містити все що стосується птахів.

PS: абсолютно не знаюся на шарпі і юніті. Поради суто архітектурні. Можливо деякі, а може й всі, будуть не будуть пасувати до Вашого прикладу.

ну то є таке, бо класи, що наслідуються від Monobehaviour'у не мона створювати через new, їх мона додавати до об'єктів за допомогою transform.AddComponent<T>(), або в редакторі прикріпити до об'єкту, а потім отримати за допомогою transform.GetComponent<T>().
Це в юніті там якась компонентна модель архітектури, чи якось так.
І сама людина - це вже об'єкт зі скриптом, котрий збережений в юніті, називається - префаб.


Тоді мені по сути треба окремий скрипт для позиціювання мішені, ага? Бо зара код, котрий відповідає за це, знаходиться в MovementCtrl

11 Відповідь від iovchynnikov Востаннє редагувалося iovchynnikov (22.08.2016 10:33:20)

  • Звідки: Спочивальня Бандери
  • Дата реєстрації: 24.01.2016
  • Повідомлень: 496
    • Репутація: 589

Re: Проаналізуйте код мого проекту, будь-ласочка

FakiNyan написав:
iovchynnikov написав:

Чесно кажучи, виглядає як мішок функціонального коду обгорнутий у методи класів.
Почніть з моделювання доменних об'єктів: людини, пташки. Бо це якийсь нонсенс:

var hc = newHuman.GetComponent<HumanController>();

Я б тут очікував:

val human = new Human(args);
//abo
val human = HumanFactory.build(arg);

А потім працювати з цим Human об'єктом, який був би зв'язаний з юніті, наприклад, патерном Observer. Вийшло б досить low coupled.
Саме у Human класі має бути уся логіка, пов'язана з ним (DDD). Не робіть товстих контролерів.
Розділення на контроллери теж дивне. Варто їх розрізняти не за функціями, а за сутностями і такими, що відповідають вимозі Single responsibility principle (SOLID): GroundCtrl не має відповідати за людей (делегуйте це HumanCtrl), a MovementCtrl має увійти до BirdCtrl, котрий буде містити все що стосується птахів.

PS: абсолютно не знаюся на шарпі і юніті. Поради суто архітектурні. Можливо деякі, а може й всі, будуть не будуть пасувати до Вашого прикладу.

ну то є таке, бо класи, що наслідуються від Monobehaviour'у не мона створювати через new, їх мона додавати до об'єктів за допомогою transform.AddComponent<T>(), або в редакторі прикріпити до об'єкту, а потім отримати за допомогою transform.GetComponent<T>().
Це в юніті там якась компонентна модель архітектури, чи якось так.
І сама людина - це вже об'єкт зі скриптом, котрий збережений в юніті, називається - префаб.


Тоді мені по сути треба окремий скрипт для позиціювання мішені, ага? Бо зара код, котрий відповідає за це, знаходиться в MovementCtrl

Це воно у юніті називається префаб, деталі імплементації, а у контексті домену (вашої гри) це - class Human.

Один з можливих варіянтів.

object UnitiHumanFactory {
  def create (args) {
     val prefab = unity.createPrefab(args.prefabData)
     val prefabHumanObserver = HumanObserverPrefab(prefab)
     val human = Human(args.humanData)
     human.addObserver(prefabHumanObserver)

     return human
  }
}

class HumanObserverPrefab {
   ..
   def onMovement(distance : Long) {
      prefab.setPos(50)
   }
}

class Human {
  def exampleMove(distance : Long) {
     pos += distance
     observers.notifyMovement(distance)
  }
}

class HumanController {
  ...... {
     var human = UnitiHumanFactory.create(myArgs)
     human.exampleMove(50)
  }
}

12 Відповідь від FakiNyan

  • FakiNyan
  • FakiNyan
  • Робітниця.
  • Поза форумом
  • Дата реєстрації: 27.06.2013
  • Повідомлень: 14 116
    • Репутація: 5674

Re: Проаналізуйте код мого проекту, будь-ласочка

я поки шо вирішив розкулупати код одного плагіну, аби глянути, як нормальні люди пишуть код

13 Відповідь від quez

  • quez
  • quez
  • Replace Team
  • Поза форумом
  • Звідки: Новий Єрусалим
  • Дата реєстрації: 01.05.2014
  • Повідомлень: 3 103
    • Репутація: 1621

Re: Проаналізуйте код мого проекту, будь-ласочка

iovchynnikov написав:

Чесно кажучи, виглядає як мішок функціонального коду обгорнутий у методи класів.
Почніть з моделювання доменних об'єктів: людини, пташки. Бо це якийсь нонсенс:

var hc = newHuman.GetComponent<HumanController>();

Я б тут очікував:

val human = new Human(args);
//abo
val human = HumanFactory.build(arg);

А потім працювати з цим Human об'єктом, який був би зв'язаний з юніті, наприклад, патерном Observer. Вийшло б досить low coupled.
Саме у Human класі має бути уся логіка, пов'язана з ним (DDD). Не робіть товстих контролерів.
Розділення на контроллери теж дивне. Варто їх розрізняти не за функціями, а за сутностями і такими, що відповідають вимозі Single responsibility principle (SOLID): GroundCtrl не має відповідати за людей (делегуйте це HumanCtrl), a MovementCtrl має увійти до BirdCtrl, котрий буде містити все що стосується птахів.

PS: абсолютно не знаюся на шарпі і юніті. Поради суто архітектурні. Можливо деякі, а може й всі, будуть не будуть пасувати до Вашого прикладу.

Слово Controller у назві класів не дає побачити, що то зовсім не контроллер із MVC. Це більше схоже на модель, зверніть увагу на поля цих класів і (особливо) на метод Update().

А ще ви очікуєте побачити фабрику гуманів, і ви її маєте:

GameObject newHuman = Instantiate(humanPref);
Подякували: FakiNyan1

14 Відповідь від iovchynnikov

  • Звідки: Спочивальня Бандери
  • Дата реєстрації: 24.01.2016
  • Повідомлень: 496
    • Репутація: 589

Re: Проаналізуйте код мого проекту, будь-ласочка

quez написав:
iovchynnikov написав:

Чесно кажучи, виглядає як мішок функціонального коду обгорнутий у методи класів.
Почніть з моделювання доменних об'єктів: людини, пташки. Бо це якийсь нонсенс:

var hc = newHuman.GetComponent<HumanController>();

Я б тут очікував:

val human = new Human(args);
//abo
val human = HumanFactory.build(arg);

А потім працювати з цим Human об'єктом, який був би зв'язаний з юніті, наприклад, патерном Observer. Вийшло б досить low coupled.
Саме у Human класі має бути уся логіка, пов'язана з ним (DDD). Не робіть товстих контролерів.
Розділення на контроллери теж дивне. Варто їх розрізняти не за функціями, а за сутностями і такими, що відповідають вимозі Single responsibility principle (SOLID): GroundCtrl не має відповідати за людей (делегуйте це HumanCtrl), a MovementCtrl має увійти до BirdCtrl, котрий буде містити все що стосується птахів.

PS: абсолютно не знаюся на шарпі і юніті. Поради суто архітектурні. Можливо деякі, а може й всі, будуть не будуть пасувати до Вашого прикладу.

Слово Controller у назві класів не дає побачити, що то зовсім не контроллер із MVC. Це більше схоже на модель, зверніть увагу на поля цих класів і (особливо) на метод Update().

А ще ви очікуєте побачити фабрику гуманів, і ви її маєте:

GameObject newHuman = Instantiate(humanPref);

GameObject != Human
Чому "модель" має постфікс Controller?

15 Відповідь від quez

  • quez
  • quez
  • Replace Team
  • Поза форумом
  • Звідки: Новий Єрусалим
  • Дата реєстрації: 01.05.2014
  • Повідомлень: 3 103
    • Репутація: 1621

Re: Проаналізуйте код мого проекту, будь-ласочка

GameObject != Human

Так, це його нащадок

Чому "модель" має постфікс Controller?

Чесно кажучи, вперше про це задумався тоді, коли прочитав перший ваш пост. Видно, так склалось історично. Фразі "весь код, який зв’язаний з цим об’єктом" не придумали кращого синоніму.

Подякували: FakiNyan1

16 Відповідь від FakiNyan

  • FakiNyan
  • FakiNyan
  • Робітниця.
  • Поза форумом
  • Дата реєстрації: 27.06.2013
  • Повідомлень: 14 116
    • Репутація: 5674

Re: Проаналізуйте код мого проекту, будь-ласочка

iovchynnikov написав:
quez написав:
iovchynnikov написав:

Чесно кажучи, виглядає як мішок функціонального коду обгорнутий у методи класів.
Почніть з моделювання доменних об'єктів: людини, пташки. Бо це якийсь нонсенс:

var hc = newHuman.GetComponent<HumanController>();

Я б тут очікував:

val human = new Human(args);
//abo
val human = HumanFactory.build(arg);

А потім працювати з цим Human об'єктом, який був би зв'язаний з юніті, наприклад, патерном Observer. Вийшло б досить low coupled.
Саме у Human класі має бути уся логіка, пов'язана з ним (DDD). Не робіть товстих контролерів.
Розділення на контроллери теж дивне. Варто їх розрізняти не за функціями, а за сутностями і такими, що відповідають вимозі Single responsibility principle (SOLID): GroundCtrl не має відповідати за людей (делегуйте це HumanCtrl), a MovementCtrl має увійти до BirdCtrl, котрий буде містити все що стосується птахів.

PS: абсолютно не знаюся на шарпі і юніті. Поради суто архітектурні. Можливо деякі, а може й всі, будуть не будуть пасувати до Вашого прикладу.

Слово Controller у назві класів не дає побачити, що то зовсім не контроллер із MVC. Це більше схоже на модель, зверніть увагу на поля цих класів і (особливо) на метод Update().

А ще ви очікуєте побачити фабрику гуманів, і ви її маєте:

GameObject newHuman = Instantiate(humanPref);

GameObject != Human
Чому "модель" має постфікс Controller?

бо воно контролює створення хуманів

17 Відповідь від iovchynnikov

  • Звідки: Спочивальня Бандери
  • Дата реєстрації: 24.01.2016
  • Повідомлень: 496
    • Репутація: 589

Re: Проаналізуйте код мого проекту, будь-ласочка

FakiNyan написав:
iovchynnikov написав:
quez написав:

Слово Controller у назві класів не дає побачити, що то зовсім не контроллер із MVC. Це більше схоже на модель, зверніть увагу на поля цих класів і (особливо) на метод Update().

А ще ви очікуєте побачити фабрику гуманів, і ви її маєте:

GameObject != Human
Чому "модель" має постфікс Controller?

бо воно контролює створення хуманів

Але модель не має за це відповідати :) Контроллер - так.

18 Відповідь від quez

  • quez
  • quez
  • Replace Team
  • Поза форумом
  • Звідки: Новий Єрусалим
  • Дата реєстрації: 01.05.2014
  • Повідомлень: 3 103
    • Репутація: 1621

Re: Проаналізуйте код мого проекту, будь-ласочка

Давайте все ж забудемо слово "модель" в контексті Юніті, бо воно збиває з пантелику. Я таки знайшов описання парадигми Юніті:  https://en.wikipedia.org/wiki/Entity–component–system .