From 17196fc564dc23fa1acb592410d9694bbe3fe341 Mon Sep 17 00:00:00 2001 From: gODealOAple Date: Fri, 24 May 2024 08:14:22 +0500 Subject: [PATCH] review --- DarkPortal/Assets/C# scripts/Camera.cs | 10 ++++++--- DarkPortal/Assets/C# scripts/Entity.cs | 4 ++-- DarkPortal/Assets/C# scripts/Fight.cs | 22 ++++++++++++++----- DarkPortal/Assets/C# scripts/FortuneWhiil.cs | 14 +++++++++--- DarkPortal/Assets/C# scripts/Player/Health.cs | 3 ++- DarkPortal/Assets/C# scripts/Player/Player.cs | 5 ++++- .../Assets/C# scripts/Player/audioSpeach.cs | 3 ++- DarkPortal/Assets/C# scripts/QUests/Quests.cs | 5 +++-- DarkPortal/Assets/ShopController.cs | 14 +++++++++--- DarkPortal/Assets/door.cs | 1 + 10 files changed, 60 insertions(+), 21 deletions(-) diff --git a/DarkPortal/Assets/C# scripts/Camera.cs b/DarkPortal/Assets/C# scripts/Camera.cs index 0889eef7..6b8be953 100644 --- 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 index 2434144e..cd0c2afa 100644 --- 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 InventoryForWeapons; private Skills skill; - public List InventoryList; + public List 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 index 2b7eefe8..7e01b71b 100644 --- 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 index 2ba5bd35..bc5682b7 100644 --- 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, + // но я бы лучше сделал Dictionary, а потом его инвертировал в Dictionary. Так будет легче настроить 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 index 02f8f59b..67c59097 100644 --- 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().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 index 711e168c..7d341592 100644 --- 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 index a4f1d17b..4d719254 100644 --- 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 index a3f873d9..9135819e 100644 --- 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 index 0ffa89d6..32076db6 100644 --- 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