SK0LX · gODeaLoAple · May 24, 2024
diff --git a/DarkPortal/Assets/C# scripts/Camera.cs b/DarkPortal/Assets/C# scripts/Camera.cs
@@ -6,8 +6,10 @@
 public class FollowPlayer : MonoBehaviour
 {
     public Transform player;
-    private bool fight;
-
+    private bool fight; // review(24.05.2024): Поле вроде никогда не меняется
+
+    // review(24.05.2024): Было бы здорово выделить структуру record CameraLimits(Transform Left, Transform Right);
+    // review(24.05.2024): Тогда будет удобно определять локацию
     public Transform leftLimit1;
     public Transform rightLimit1;
 
@@ -28,6 +30,8 @@ private void Start()
 
     private void LateUpdate()
     {
+        // review(24.05.2024): Это все можно заметно упростить
+        // if (player.position.IsBetweenX(leftLimit1, rightLimit1)) transform.position = CameraLocate(leftLimit1, rightLimit1); и т.д.
         if (player.position.x < rightLimit1.position.x && player.position.x > leftLimit1.position.x)
             location = 1;
         if (player.position.x < rightLimit2.position.x && player.position.x > leftLimit2.position.x)
@@ -67,7 +71,7 @@ private void OnDrawGizmos()
     private Vector3 CameraLocate(float min, float max)
     {
         return new Vector3(
-            Mathf.Clamp(player.position.x, min + 7, max - 7),
+            Mathf.Clamp(player.position.x, min + 7, max - 7), // review(24.05.2024): Давайте избавимся от магических констант
             player.position.y,
             -5);
     }

diff --git a/DarkPortal/Assets/C# scripts/Entity.cs b/DarkPortal/Assets/C# scripts/Entity.cs
@@ -21,7 +21,7 @@ public class Entity: MonoBehaviour
     private RectTransform positionCanvas;
     // public List<int> InventoryForWeapons;
     private Skills skill;
-    public List<PlayerInventory> InventoryList; 
+    public List<PlayerInventory> InventoryList; // review(24.05.2024): Это поле точно нужно?
 
     public void Start()
     {
@@ -41,7 +41,7 @@ public void Update()
 
     public void TakeDamage(int damage)
     {
-        health -= damage;
+        health -= damage; // review(24.05.2024): Тут хорошо бы сделать Math.Max(0, health - damage);
         UpdateCanvasHp();
     }
 

diff --git a/DarkPortal/Assets/C# scripts/Fight.cs b/DarkPortal/Assets/C# scripts/Fight.cs
@@ -13,7 +13,7 @@ public class Fight : MonoBehaviour
     private Entity enemyComponent;
 
     private bool facingRight;
-    public bool isPlayerTurn;
+    public bool isPlayerTurn; // review(24.05.2024): Это поле дублирует поле игрока
     private bool isBossFight;
     private bool buttonClick;
     public Canvas canvas;
@@ -54,7 +54,7 @@ void Update()
         }
     }
 
-
+    // review(24.05.2024): Choose first step fighter 
     private void ChooseRandomMove() // типо монетку подбрасываем в начале файта
     {
         var rnd = new Random();
@@ -67,9 +67,9 @@ private int ChooseRandomDamage(int downDamage, int upDamage) // машина п
         critDamage = false;
         if (upDamage - downDamage < 0)
             throw new ArgumentException("Верхний предел урона не может быть ниже нижнего предела урона");
-        var rnd = new Random();
+        var rnd = new Random(); // review(24.05.2024): Может, создать один Random и поместить его в поле?
         var damage = rnd.Next(downDamage,upDamage + 1);
-        if (rnd.Next(0, 101) < 20)
+        if (rnd.Next(0, 101) < 20) // review(24.05.2024): Как будто не хватает exntension-метода rnd.FlipCoin(int percent)
         {
             Debug.Log(!isPlayerTurn ? $"CRIT mob={enemy.name}" : $"CRIT player");
             damage += (upDamage-downDamage) / 2;
@@ -81,10 +81,21 @@ private int ChooseRandomDamage(int downDamage, int upDamage) // машина п
 
     private IEnumerator CoreFight() //сам весь процесс файта (очереди)
     {
+        // review(24.05.2024): Было бы круто выделить методы RunPlayerStep/RunEnemyStep
         if (isPlayerTurn)
         {
+            // review(24.05.2024): Долго пытался понять, что такое buttonClick. Потом осознал, что это clickGuard (типа того). Стоит переименовать, чтобы было понятно.
+            // А еще лучше, наверное, вынести эту проверку вне if-ов, чтобы было так:
+            // if (!clickGuard)
+            // {
+            //    clickGuard = true;
+            //    // logic
+            //    clickGuard = false;
+            // }
+            // Но мне кажется, что есть менее костыльные способы реализовать эту логику
             if (!buttonClick && (Input.GetKeyDown(KeyCode.Z) || player.activateButtonForAttack))
             {
+                // review(24.05.2024): Стоит выделить методы PlayPlayerAttackStep/PlayPlayerHealingStep
                 buttonClick = true;
                 yield return StartCoroutine(player.Attack());
                 enemyComponent.TakeDamage(ChooseRandomDamage(10, 15));
@@ -129,7 +140,8 @@ private IEnumerator CoreFight() //сам весь процесс файта (о
             }
         }
     }
-
+
+    // review(24.05.2024): Почему не bool?
     private int ChooseDamageSkip(int dexterity) // поможет ли ловкость уйти от урона, хммм
     {
         var rnd = new Random();

diff --git a/DarkPortal/Assets/C# scripts/FortuneWhiil.cs b/DarkPortal/Assets/C# scripts/FortuneWhiil.cs
@@ -6,7 +6,7 @@
 using UnityEngine.UI;
 using Random = UnityEngine.Random;
 
-
+// review(24.05.2024): FortuneWheel
 public class FortuneWhheel : MonoBehaviour
 {
     private int numberOfTurnes; //кол-во оборотов
@@ -17,7 +17,7 @@ public class FortuneWhheel : MonoBehaviour
     private bool canWeTurn;
 
     private string winningText;
-    private string totalChoiseText;
+    private string totalChoiseText; // review(24.05.2024): choice
 
 
     public Player player;
@@ -72,6 +72,7 @@ private IEnumerator TurnTheWheel()
         numberOfTurnes = Random.Range(200, 300);
         speed = 0.01f;
         melstroi.Play();
+        // review(24.05.2024): Я бы вращение выделил в метод
         for (var i = 0; i < numberOfTurnes; i++)
         {
             transform.Rotate(0, 0, 12f);
@@ -90,12 +91,16 @@ private IEnumerator TurnTheWheel()
                 speed = 0.09f;
             }
 
+            // review(24.05.2024): Создается много одинаковых объектов. Я бы рекомендовал предсоздать WaitForSeconds для всех кейсов (либо забить)
             yield return new WaitForSeconds(speed);
         }
 
         melstroi.Stop();
         WhatWeWin = Mathf.RoundToInt(transform.rotation.eulerAngles.z) / 12;
 
+        // review(24.05.2024): Давай выделим enum, либо константы для строк.
+        // review(24.05.2024): А еще можно значительно сократить код, если сделаем, например, Dictionary<int, string>,
+        // но я бы лучше сделал Dictionary<string, int[]>, а потом его инвертировал в Dictionary<int, string>. Так будет легче настроить
         switch (WhatWeWin)
         {
             case 0:
@@ -219,6 +224,7 @@ private void CheckWinOrLose()
 
     private void PressBtnUp()
     {
+        // review(24.05.2024): Еще можно так totalBet = Math.Min(totalBet + 10, MaxBet);
         totalBet += 10;
         if (totalBet > 100)
             totalBet = 100;
@@ -228,13 +234,15 @@ private void PressBtnUp()
 
     private void PressBtnDown()
     {
+        // review(24.05.2024): Еще можно так totalBet = Math.Max(totalBet - 10, MinBet);
+        // Кстати, это норм, что ставка может быть 0?
         totalBet -= 10;
         if (totalBet < 0)
             totalBet = 0;
         totalBetText.text = totalBet.ToString();
     }
 
-
+    // review(24.05.2024): Я бы попробовал избавиться от дублирования за счет выделения структуры PlayerChoice(string Text, int Multiplier);
     private void PressGreen()
     {
         if (canWeTurn)

diff --git a/DarkPortal/Assets/C# scripts/Player/Health.cs b/DarkPortal/Assets/C# scripts/Player/Health.cs
@@ -18,8 +18,9 @@ public class Health : MonoBehaviour
 
     public void TakeHit(int damage)
     {
-        health -= damage;
+        health -= damage; // review(24.05.2024): Сюда стоит вставить Math.Max(0, health - damage);
         hit.Play();
+        // review(24.05.2024): Компонент HealthBar стоит захватить в void Awake() и поместить в поле
         gameObject.GetComponent<HealthBar>().fill = (float)health / maxHealth;
 
         if (health <= 0)

diff --git a/DarkPortal/Assets/C# scripts/Player/Player.cs b/DarkPortal/Assets/C# scripts/Player/Player.cs
@@ -18,6 +18,8 @@ public class Player : MonoBehaviour
     public bool fight;
     public Canvas canvasDefault;
     public PlayerInventory inventory;
+
+    // review(24.05.2024): Кнопки можно выделить в отдельную компоненту PlayerGui
     public Button buttonForAttack;
     public bool activateButtonForAttack;
     public Button buttonForHealth;
@@ -132,7 +134,8 @@ public IEnumerator HealingPlayer()
         Animator.SetTrigger("default");
         heal.Play();
     }
-
+
+    // review(24.05.2024): Код дублируется в Fight
     private int ChooseRandomHealth(int downHealth, int upHealth) // машина по рандомизированному хп(крит 20%)
     {
         if (upHealth - downHealth < 0)

diff --git a/DarkPortal/Assets/C# scripts/Player/audioSpeach.cs b/DarkPortal/Assets/C# scripts/Player/audioSpeach.cs
@@ -16,7 +16,7 @@ public class audioSpeach : MonoBehaviour
     // Update is called once per frame
     private void Update()
     {
-        if (speak && player.speed >= 1e-6)
+        if (speak && player.speed >= 1e-6) // review(24.05.2024): Почему не >= 0?
         {
             StartCoroutine(SpeakKnight());
         }
@@ -26,6 +26,7 @@ private IEnumerator SpeakKnight()
     {
         speak = false;
         yield return new WaitForSeconds(30);
+        // review(24.05.2024): Можно все knightN положить в массив и сделать так: knights[rnd.Next(knights.Length)].Play();
         var rnd = new Random();
         switch (rnd.Next(0, 5))
         {

diff --git a/DarkPortal/Assets/C# scripts/QUests/Quests.cs b/DarkPortal/Assets/C# scripts/QUests/Quests.cs
@@ -32,14 +32,15 @@ public void StartFirstQuest(string textForButton, string[] textForAllInformation
     {
         firstQuestTextForButton.text = textForButton;
         firstQuest.gameObject.SetActive(true);
-        var text = "";
+        var text = ""; // review(24.05.2024): StringBuilder
         for (var i = 0; i < textForAllInformation.Length; i++)
         {
             text += $"{i + 1}. {textForAllInformation[i]}\n";
         }
         firstQuestTextAll.text = text;
     }
-
+
+    // review(24.05.2024): Метод почти полностью дублирует StartFirstQuest
     public void StartSecondQuest(string textForButton, string[] textForAllInformation)
     {
         secondQuestTextForButton.text = textForButton;

diff --git a/DarkPortal/Assets/ShopController.cs b/DarkPortal/Assets/ShopController.cs
@@ -8,17 +8,19 @@
 public class ShopController : MonoBehaviour
 {
     public Player player;
-    private int coinsPLayer;
-
+    private int coinsPLayer; // review(24.05.2024): Переменная как будто не используется
+
+    // review(24.05.2024): Поля точно должны быть публичными?
     public Button buttonForSmallHP;
     public Button buttonForBigHP;
     public Button buttonForDecorationHP;
     public bool buy1HP;
     public Button buttonForDecorationDeterity;
     public bool buy2Deterity;
-    [SerializeField] private AudioSource noMonie;
+    [SerializeField] private AudioSource noMonie; // review(24.05.2024): noMoneyAudioSource
     void Start()
     {
+        // review(24.05.2024): Вы берете компоненту кнопки, потому что в кнопке кнопка?
         buttonForSmallHP.GetComponent<Button>().onClick.AddListener(HpSmall);
 
         buttonForBigHP.GetComponent<Button>().onClick.AddListener(HpBig);
@@ -30,6 +32,7 @@ void Start()
 
     private void Update()
     {
+        // review(24.05.2024): не совсем понятна логика. Это нужно, чтобы игрок не мог купить много HP?
         if (buy1HP)
         {
 
@@ -43,6 +46,11 @@ private void Update()
         }
     }
 
+    // review(24.05.2024): Повторяется логика
+    // 1. Если у игрока не хватает денег, просто проиграй звук
+    // 2. Иначе уменьши количество денег игрока и сделай что-то
+    // Предлагаю все, кроме "сделай что-то" вынести в общий код. Например, можно использовать структуру
+    // public record ShopItem(int Price, Action OnBuyAction);
     void HpSmall()
     {
         if (player.inventory.coins >= 7)

diff --git a/DarkPortal/Assets/door.cs b/DarkPortal/Assets/door.cs
@@ -5,6 +5,7 @@
 using UnityEngine.Rendering;
 using UnityEngine.UI;
 
+// review(24.05.2024): Стои все классы называть с заглавной буквы
 public class door : MonoBehaviour
 {
     public Canvas canvasForTavernaBtn;