diff --git a/Assets/Scripts/BreakBars.cs b/Assets/Scripts/BreakBars.cs index 0796105..90eec9d 100644 --- a/Assets/Scripts/BreakBars.cs +++ b/Assets/Scripts/BreakBars.cs @@ -20,16 +20,16 @@ void Update() if (GameState.IsOverGameKeyboard && GameState.HaveCrowbar) interactableObject.isInteractable = true; - if (GameState.IsOverGameKeyboard + if (GameState.IsOverGameKeyboard // review(27.06.2024): не совсем понятно, почему тут эта проверка. Наверное, стоило написать что-то типа "CanInteract()" метода && GameState.HaveCrowbar && isTriggered && Input.GetKeyDown(KeyCode.E) && !GameState.CanOpenToiletDoor) { - audioManager.PlaySFX(audioManager.brokenBars); + audioManager.PlaySFX(audioManager.brokenBars); // review(27.06.2024): Т.к. тут используются поля audioManager, то стоило просто отдельный метод выделить, либо enum использовать brokenBars.SetActive(true); wholeBars.SetActive(false); - StartCoroutine(Wait(0.5f)); + StartCoroutine(Wait(0.5f)); // review(27.06.2024): Магическая константа. Стоило вынести в поле } } @@ -39,7 +39,8 @@ private IEnumerator Wait(float time) GameState.CanOpenToiletDoor = true; GameState.ChecksBool.Add(DialogFlagEnum.BrokenBars); } - + + // review(27.06.2024): Код в очередной раз дублируется private void OnTriggerEnter2D(Collider2D other) { if (!other.CompareTag("Player")) diff --git a/Assets/Scripts/ConnectDots/DotsManager.cs b/Assets/Scripts/ConnectDots/DotsManager.cs index dcf5daf..5906daa 100644 --- a/Assets/Scripts/ConnectDots/DotsManager.cs +++ b/Assets/Scripts/ConnectDots/DotsManager.cs @@ -31,7 +31,7 @@ public class DotsManager : MonoBehaviour [SerializeField] private CanvasGroup tipsCanvas; - private readonly List CompletedTiles = new(); + private readonly List CompletedTiles = new(); // review(17.06.2024): Приватные поля с маленькой буквы private bool isCoroutineStarted; private void Start() diff --git a/Assets/Scripts/ConnectDots/DotsTile.cs b/Assets/Scripts/ConnectDots/DotsTile.cs index 98b8202..1c7d56e 100644 --- a/Assets/Scripts/ConnectDots/DotsTile.cs +++ b/Assets/Scripts/ConnectDots/DotsTile.cs @@ -5,16 +5,16 @@ public class DotsTile : MonoBehaviour { public DotsManager dotsManager; - public bool isHeadTile; + public bool isHeadTile; // review(17.06.2024): Как будто isEmpty звучит понятнее private SpriteRenderer sprite; private Color endColor; - public List adjacentTiles; + public List adjacentTiles; // review(17.06.2024): Очень надеюсь, что вы не руками проставляли adjacentTiles... Кажется, что саму карту можно было построить из кода (что было бы даже проще) private void Awake() { sprite = GetComponent(); dotsManager.defaultColor = sprite.color; - dotsManager.UncompletedTiles.Add(sprite); + dotsManager.UncompletedTiles.Add(sprite); // review(17.06.2024): Тут как раз нарушается MVP: вы основываете решение о незаконченных тайлах на основе их цвета, а не строите внутреннюю модель if (isHeadTile) endColor = transform.GetChild(0).GetComponent().color; } @@ -27,6 +27,7 @@ private void OnMouseOver() if (dotsManager.isClicked) { sprite.color = dotsManager.currentColor; + // review(17.06.2024): Всю логику dotsManager-а можно было бы запихнуть внутрь dotsManager.Click(gameObject) dotsManager.CurrentTiles.Add(sprite); dotsManager.prevTile = gameObject; dotsManager.isStarted = true; @@ -57,6 +58,7 @@ private void OnMouseOver() } } + // review(17.06.2024): Всю логику ниже стоило поместить в DotsManager if (dotsManager.CurrentTiles.Count > 1 && sprite == dotsManager.CurrentTiles[^2] && dotsManager.isClicked) { dotsManager.prevTile = dotsManager.CurrentTiles[^2].gameObject; diff --git a/Assets/Scripts/ConnectDots/EraseButton.cs b/Assets/Scripts/ConnectDots/EraseButton.cs index 7a4c561..ef2295c 100644 --- a/Assets/Scripts/ConnectDots/EraseButton.cs +++ b/Assets/Scripts/ConnectDots/EraseButton.cs @@ -7,7 +7,7 @@ public class EraseButton : MonoBehaviour { public DotsManager dotsManager; - void EraseField() + void EraseField() // review(17.06.2024): Метод не используется, можно удалить { if (!GameState.IsOverGameWires) dotsManager.EraseField(); diff --git a/Assets/Scripts/ConnectDots/GlobalLightState.cs b/Assets/Scripts/ConnectDots/GlobalLightState.cs index c480431..02309fe 100644 --- a/Assets/Scripts/ConnectDots/GlobalLightState.cs +++ b/Assets/Scripts/ConnectDots/GlobalLightState.cs @@ -7,6 +7,6 @@ public class GlobalLightState : MonoBehaviour { [SerializeField] private Light2D globalLight; - public void TurnOffLight() + public void TurnOffLight() // review(17.06.2024): Метод не используется, можно удалить => globalLight.intensity = 0; } \ No newline at end of file diff --git a/Assets/Scripts/CraftItem.cs b/Assets/Scripts/CraftItem.cs index 933d070..e0216da 100644 --- a/Assets/Scripts/CraftItem.cs +++ b/Assets/Scripts/CraftItem.cs @@ -20,7 +20,7 @@ void Start() void Update() { var hasBrickAndNail = Inventory.PlayerInventory.ContainsKey(ItemName.Nail) && - Inventory.PlayerInventory.ContainsKey(ItemName.Brick); + Inventory.PlayerInventory.ContainsKey(ItemName.Brick); // review(27.06.2024): Уместнее было инкапсулировать логику проверки вещей в метод Inventory.Contains(...) if (hasBrickAndNail) interactableObject.isInteractable = true; diff --git a/Assets/Scripts/DoorPassage.cs b/Assets/Scripts/DoorPassage.cs index e2b6d36..ed8bdff 100644 --- a/Assets/Scripts/DoorPassage.cs +++ b/Assets/Scripts/DoorPassage.cs @@ -17,7 +17,7 @@ private void Start() private void Update() { - if (doorType == Door.RoomDoor && GameState.IsOverGameWires) + if (doorType == Door.RoomDoor && GameState.IsOverGameWires) // review(27.06.2024): Второй операнд лишний, наверное interactableObject.isInteractable = GameState.IsOverGameWires; if (doorType == Door.ToiletDoor) interactableObject.isInteractable = GameState.CanOpenToiletDoor; @@ -25,11 +25,14 @@ private void Update() interactableObject.isInteractable = GameState.HaveCrowbar; if (isTriggered && Input.GetKeyDown(KeyCode.E)) { + // review(27.06.2024): Дублируется код. Тут имело смысл предсоздать объект(ы) типа DoorModel(Door Id, Func CanEnter) + // тогда упростилась бы и верхняя проверка, и эта if ((GameState.IsOverGameWires && doorType == Door.RoomDoor) || (GameState.CanOpenToiletDoor && doorType == Door.ToiletDoor) || (GameState.HaveCrowbar && doorType == Door.Ventilation)) { audioManager.PlaySFX(audioManager.door); + // review(27.06.2024): код ниже должен стать одним методом у Player var vector3 = player.transform.position; var position = exitDoor.transform.position; vector3.x = position.x; diff --git a/Assets/Scripts/ElectricalPanel.cs b/Assets/Scripts/ElectricalPanel.cs index 8c9f159..40151b6 100644 --- a/Assets/Scripts/ElectricalPanel.cs +++ b/Assets/Scripts/ElectricalPanel.cs @@ -17,7 +17,7 @@ private void Start() private void Update() { - interactableObject.isInteractable = GameState.ActivePlayer.penguinName == PenguinNames.Estriper; + interactableObject.isInteractable = GameState.ActivePlayer.penguinName == PenguinNames.Estriper; // review(27.06.2024): CanInteract(Player player) if (isTriggered && Input.GetKeyDown(KeyCode.E)) { showGame = !showGame; @@ -37,6 +37,7 @@ private void OnTriggerExit2D(Collider2D other) if (other.CompareTag("Player")) { isTriggered = false; + // review(27.06.2024): Все, что связано с dots стоило выделить в метод DotsManager dots.isClicked = false; dots.gameObject.SetActive(false); interactableObject.isInteractable = false; diff --git a/Assets/Scripts/EndToilet.cs b/Assets/Scripts/EndToilet.cs index 37bb608..bcf0c77 100644 --- a/Assets/Scripts/EndToilet.cs +++ b/Assets/Scripts/EndToilet.cs @@ -41,7 +41,8 @@ private void Update() inventory.SetActive(false); GameState.ActivePlayer.isActive = false; - + + // review(27.06.2024): Я бы создал дикт PenguinNames -> Animator и упростил логику if (GameState.ActivePlayer.penguinName != PenguinNames.Cago) CagoAnimator.SetBool("IsGameOver", true); @@ -60,7 +61,7 @@ private void Update() private IEnumerator WaitTime() { GameState.ChecksBool.Add(DialogFlagEnum.End); - while (Math.Abs(EndCanvas.alpha - 1) >= 1e-9) + while (Math.Abs(EndCanvas.alpha - 1) >= 1e-9) // review(27.06.2024): Как будто не хватает extension-метода static bool IsEqualTo(this float number, float other, float accuracy = 1e-6) { EndCanvas.alpha += 0.005f; yield return null; diff --git a/Assets/Scripts/GameState.cs b/Assets/Scripts/GameState.cs index 8839727..aa40f0b 100644 --- a/Assets/Scripts/GameState.cs +++ b/Assets/Scripts/GameState.cs @@ -5,14 +5,14 @@ public static class GameState { public static bool IsOverGameWires; public static bool IsOverGameKeyboard; - public static bool HaveCrowbar; + public static bool HaveCrowbar; // review(27.06.2024): HasCrowbar? А еще мб стоило честную инвентарную систему сделать? public static bool IsNowTextDisplayed; public static bool IsOpenKeyboardGame; public static bool CanOpenToiletDoor; public static Player ActivePlayer; public static Vector3 PlatformerSpawn = new(0 , 0, 0); - public static HashSet ChecksBool = new() + public static HashSet ChecksBool = new() // review(26.06.2024): Кмк более понятнее было бы ActivatedDialogs { DialogFlagEnum.Ventilation, DialogFlagEnum.None, @@ -20,5 +20,7 @@ public static class GameState DialogFlagEnum.KeyboardForStupid }; - public static AudioClip CurrentMusic; + // review(26.06.2024): Да, удобно, но кмк GameState - это бизнес-логика, а она не должна зависеть от моделей View (звуки - это тоже View) + // review(26.06.2024): Можно было бы завести enum под каждый стиль музыки и в классе создать дикт: enum -> AudioClip + public static AudioClip CurrentMusic; } \ No newline at end of file diff --git a/Assets/Scripts/InteractableObject.cs b/Assets/Scripts/InteractableObject.cs index 95902fd..aed4eff 100644 --- a/Assets/Scripts/InteractableObject.cs +++ b/Assets/Scripts/InteractableObject.cs @@ -23,7 +23,7 @@ private void OnTriggerEnter2D(Collider2D other) { if (isInteractable && other.CompareTag("PlayerTrigger") && other.transform.parent.GetComponent().isActive) { - other.GetComponent().triggeredInteractableObjects.Add(gameObject); + other.GetComponent().triggeredInteractableObjects.Add(gameObject); // review(27.06.2024): Не хватает инкапсуляции логики sr.material = highlightMaterial; } } diff --git a/Assets/Scripts/Inventory.cs b/Assets/Scripts/Inventory.cs index 23ecf1a..42d87e5 100644 --- a/Assets/Scripts/Inventory.cs +++ b/Assets/Scripts/Inventory.cs @@ -3,7 +3,7 @@ using System.Collections.Generic; using UnityEngine; -public class Inventory : MonoBehaviour +public class Inventory : MonoBehaviour // review(27.06.2024): А зачем отнаследовали статический класс от MonoBehaviour? { public static Dictionary PlayerInventory; diff --git a/Assets/Scripts/InventoryUI.cs b/Assets/Scripts/InventoryUI.cs index 9bd9304..7833b55 100644 --- a/Assets/Scripts/InventoryUI.cs +++ b/Assets/Scripts/InventoryUI.cs @@ -7,7 +7,7 @@ public class InventoryUI : MonoBehaviour private void Start() { - Inventory.OnItemChangedCallback += UpdateUI; + Inventory.OnItemChangedCallback += UpdateUI; // review(27.06.2024): О, проявление MVC slots = transform.GetComponentsInChildren(); } @@ -15,7 +15,7 @@ private void UpdateUI() { Debug.Log("Updating UI"); Debug.Log(Inventory.PlayerInventory); - var playerInventoryValues = Inventory.PlayerInventory.Values.ToArray(); + var playerInventoryValues = Inventory.PlayerInventory.Values.ToArray(); // review(27.06.2024): Inventory.GetAll() -> Item[] for (var i = 0; i < slots.Length; i++) { if (i < Inventory.PlayerInventory.Count) diff --git a/Assets/Scripts/KeyboardGame/KBGameChecker.cs b/Assets/Scripts/KeyboardGame/KBGameChecker.cs index 7775972..4845cb8 100644 --- a/Assets/Scripts/KeyboardGame/KBGameChecker.cs +++ b/Assets/Scripts/KeyboardGame/KBGameChecker.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using UnityEngine; +// review(26.06.2024): вроде как не используется, можно было бы удалить public class KBGameChecker : MonoBehaviour { private SpriteRenderer sprite; @@ -14,6 +15,7 @@ private void Awake() private void Update() { + // review(26.06.2024): Каждый раз в update гонять проверку - накладно. Исключаем те случаи, когда вы навешиваете и убираете компонент if (GameState.IsOverGameKeyboard) sprite.color = Color.gray; } diff --git a/Assets/Scripts/KeyboardGame/KeyboardGame.cs b/Assets/Scripts/KeyboardGame/KeyboardGame.cs index f92b4a0..c5e1cc7 100644 --- a/Assets/Scripts/KeyboardGame/KeyboardGame.cs +++ b/Assets/Scripts/KeyboardGame/KeyboardGame.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using UnityEngine; +// review(26.06.2024): Вроде как класс называется KeyboardGame, а по факту он лишь включает/выключает игру. Я бы назвал его KeyboardGameSwitcher public class KeyboardGame : MonoBehaviour { public GameObject game; @@ -17,17 +18,17 @@ private void Awake() private void Update() { - interactableObject.isInteractable = GameState.ActivePlayer.penguinName == PenguinNames.Kawazaki; + interactableObject.isInteractable = GameState.ActivePlayer.penguinName == PenguinNames.Kawazaki; // review(26.06.2024): стоило вынести Kawazaki в константы класса if (isTriggered && Input.GetKeyDown(KeyCode.E)) { - game.SetActive(true); + game.SetActive(true); // review(26.06.2024): Вроде как создали InteractableObject, но все равно дублируете код активации. Возможно, имело смысл в InteractableObject положить Action, который должен выполниться GameState.IsOpenKeyboardGame = true; } } private void OnTriggerEnter2D(Collider2D other) { - if (other.CompareTag("Player") && other.GetComponent().penguinName == PenguinNames.Kawazaki) + if (other.CompareTag("Player") && other.GetComponent().penguinName == PenguinNames.Kawazaki) // review(26.06.2024): Повторение проверки. Имело смысл вынести в метод CanPlayKeyboardGame(Player player) { isTriggered = true; } diff --git a/Assets/Scripts/KeyboardGame/TextPuzzle.cs b/Assets/Scripts/KeyboardGame/TextPuzzle.cs index 4045b54..1261409 100644 --- a/Assets/Scripts/KeyboardGame/TextPuzzle.cs +++ b/Assets/Scripts/KeyboardGame/TextPuzzle.cs @@ -6,6 +6,7 @@ using TMPro; using UnityEngine.Serialization; +// review(27.06.2024): Странно, что TextPuzzle находится в пространсте KeyboardGame public class TextPuzzle : MonoBehaviour { [SerializeField] private TMP_Text text; @@ -17,16 +18,24 @@ public class TextPuzzle : MonoBehaviour [SerializeField] private CanvasGroup tipsCanvas; private string answer = "2 7 3 4"; - private static List Alphabet = new() + private static List Alphabet = new() // review(26.06.2024): private static readonly IReadOnlyList Alphabet { - KeyCode.Alpha0, KeyCode.Alpha1, KeyCode.Alpha2, KeyCode.Alpha3, - KeyCode.Alpha4, KeyCode.Alpha5, KeyCode.Alpha6, KeyCode.Alpha7, - KeyCode.Alpha8, KeyCode.Alpha9 + KeyCode.Alpha0, + KeyCode.Alpha1, + KeyCode.Alpha2, + KeyCode.Alpha3, + KeyCode.Alpha4, + KeyCode.Alpha5, + KeyCode.Alpha6, + KeyCode.Alpha7, + KeyCode.Alpha8, + KeyCode.Alpha9 }; private IEnumerator EndGame() { RedLamp.SetActive(false); + // review(26.06.2024): Стоило в GameState выделить метод OnEndTextPuzzle() или типа того и уместить туда всю эту логику GameState.IsOverGameKeyboard = true; GameState.ChecksBool.Add(DialogFlagEnum.Keyboard); if (GameState.ChecksBool.Contains(DialogFlagEnum.Crowbar)) @@ -40,6 +49,7 @@ private IEnumerator EndGame() void Start() { + // review(26.06.2024): text ??= GetComponent(); if (text == null) text = GetComponent(); } @@ -50,17 +60,18 @@ void Update() return; if (text.text == answer) { - StartCoroutine(EndGame()); + StartCoroutine(EndGame()); // review(26.06.2024): это норма, что корутина будет стартовать каждый раз, когда будет ответ? } if (Input.GetKeyDown(KeyCode.Backspace) && text.text.Length > 0) { text.text = ""; - keyAnimators[10].Play("PressKey"); + keyAnimators[10].Play("PressKey"); // review(26.06.2024): Мне не очень нравится, что keyAnimators как бы зависят от Alphabet, но формально никак не связаны. Было бы круто выделить модель типа KeyboardKey(AudioClip Sound, KeyCode Code) } if (text.text.Length >= answer.Length) return; + foreach (var el in Alphabet) { if (!Input.GetKeyDown(el)) continue; @@ -72,5 +83,18 @@ void Update() keyAnimators[newDigit - '0'].Play("PressKey"); } + + // review(26.06.2024): решил написать свою версию метода. Возможно, она вам покажется читаемее + foreach (var el in Alphabet) + { + if (!Input.GetKeyDown(el)) continue; + var nexIndex = (Alphabet.IndexOf(el) + 2) % Alphabet.Count; + if (text.text.Length != 0) + text.text += " "; + else // review(26.06.2024): у этой сущности довольно сложный сеттер, поэтому стоит как можно реже к нему обращаться + text.text += " " + nexIndex; + + keyAnimators[nexIndex].Play("PressKey"); + } } } \ No newline at end of file diff --git a/Assets/Scripts/MusicChanger.cs b/Assets/Scripts/MusicChanger.cs index 1735d80..525cc73 100644 --- a/Assets/Scripts/MusicChanger.cs +++ b/Assets/Scripts/MusicChanger.cs @@ -17,7 +17,7 @@ private void OnTriggerEnter2D(Collider2D other) { if (other.CompareTag("Player") && music != GameState.CurrentMusic) { - GameState.CurrentMusic = music; + GameState.CurrentMusic = music; // review(26.06.2024): Коль скоро вы задаете исходную музыку в самом audioManager, то стоило и изменение CurrentMusic поместить в audioManager.ChangeMusic audioManager.ChangeMusic(music); } } diff --git a/Assets/Scripts/PlatformerRoom/Killer.cs b/Assets/Scripts/PlatformerRoom/Killer.cs index 99681f8..d4dce3f 100644 --- a/Assets/Scripts/PlatformerRoom/Killer.cs +++ b/Assets/Scripts/PlatformerRoom/Killer.cs @@ -2,9 +2,25 @@ public class Killer : MonoBehaviour { + // review(26.06.2024): очень много похожего кода. На вашем месте я бы выделил базовый класс + // public abstract class PlayerTrigger : MonoBehaviour + // { + // private void OnTriggerEnter2D(Collider2D other) + // { + // if (other.CompareTag("Player") && other.GetComponent() is {} player) + // { + // OnPlayerTriggerEnter2D(player); + // } + // } + // + // public virtual void OnPlayerTriggerEnter2D(Player player) + // { + // } + // } + private void OnTriggerEnter2D(Collider2D other) { - var unit = other.GetComponent(); + var unit = other.GetComponent(); // review(26.06.2024): Хм, разве не нужно сначала проверить тег, а уже потом доставать компонент? if (unit && other.CompareTag("Player")) { unit.Die(unit); diff --git a/Assets/Scripts/PlatformerRoom/MovingObjectController.cs b/Assets/Scripts/PlatformerRoom/MovingObjectController.cs index 814f0fe..c3cef30 100644 --- a/Assets/Scripts/PlatformerRoom/MovingObjectController.cs +++ b/Assets/Scripts/PlatformerRoom/MovingObjectController.cs @@ -1,4 +1,6 @@ +using System.Numerics; using UnityEngine; +using Vector3 = UnityEngine.Vector3; public class MovingObject : MonoBehaviour { @@ -37,7 +39,8 @@ public void Awake() private void Update() { - if (DistanceX > 1e-6) + // review(26.06.2024): Я убежден, что DistanceX, DistanceY вполне можно заменить на 2d вектор + if (DistanceX > 1e-6) // review(26.06.2024): Стоило вынести константу либо метод добавить на корректную проверку == 0 MoveX(); if (DistanceY > 1e-6) MoveY(); @@ -69,6 +72,17 @@ private void MoveY() } } + // review(26.06.2024): Не поленился и сократил код до нескольких строчек. Вангую, можно пойти еще дальше и не потерять в понятности + private void MoveX2() + { + var position = transform.position; + DeltaX = (movingLeft ? -Speed : Speed) * Time.deltaTime; + + if (position.x > leftEdge && position.x < rightEdge) + transform.position = position + new Vector3(DeltaX, 0, 0); + else + movingLeft = !movingLeft; + } private void MoveX() { diff --git a/Assets/Scripts/PlatformerRoom/Press.cs b/Assets/Scripts/PlatformerRoom/Press.cs index d0ece08..098e3a3 100644 --- a/Assets/Scripts/PlatformerRoom/Press.cs +++ b/Assets/Scripts/PlatformerRoom/Press.cs @@ -18,16 +18,18 @@ private void Start() private IEnumerator MovePress() { - while (true) + // review(26.06.2024): Стоило хотя бы какой-нибудь флаг добавить в качестве условия, а то этот цикл ведь реально никогда не прекратится, а такая потребность обязательно возникнет + while (true) // review(26.06.2024): Зажали скобки, ясно if (movingDown) { + // review(26.06.2024): Стоило выделить transform.position в переменную while (transform.position.y > lowerPoint.position.y) { transform.position = Vector2.MoveTowards(transform.position, - new Vector2(transform.position.x, lowerPoint.position.y), moveSpeed * Time.deltaTime * 20); + new Vector2(transform.position.x, lowerPoint.position.y), moveSpeed * Time.deltaTime * 20); // review(26.06.2024): магическая константа. Стоило выделить moveDownSpeed/moveUpSpeed yield return null; } - yield return new WaitForSeconds(0.2f); + yield return new WaitForSeconds(0.2f); // review(26.06.2024): WaitForSeconds - это класс. Соответственно, вы часто создаете один и тот же объект. Можно выделить в статическое поле movingDown = false; } @@ -40,7 +42,7 @@ private IEnumerator MovePress() yield return null; } - yield return new WaitForSeconds(1.0f); + yield return new WaitForSeconds(1.0f); // review(26.06.2024): аналогично movingDown = true; audioSource.PlayOneShot(audioSource.clip); } diff --git a/Assets/Scripts/Player.cs b/Assets/Scripts/Player.cs index 6dac61a..660f2f7 100644 --- a/Assets/Scripts/Player.cs +++ b/Assets/Scripts/Player.cs @@ -30,6 +30,8 @@ private void Awake() // Update is called once per frame private void Update() { + // review(27.06.2024): Есть ощущение, что стоило сделать так, чтобы активный игрок и все, что с ним связано изменялось только при смене персонажа, а не на каждый update + // а сейчас получилось не очень эффективно if (isActive) { GameState.ActivePlayer = this; @@ -37,6 +39,7 @@ private void Update() if (isActive && playerTrigger.isTriggered && Input.GetKeyDown(KeyCode.F)) { + // review(27.06.2024): Методы, связанные с playerTrigger стоило инкапсулировать playerTrigger.gameObject.SetActive(false); isActive = false; playerTrigger.otherPlayer.isActive = true; diff --git a/Assets/Scripts/PlayerController.cs b/Assets/Scripts/PlayerController.cs index a69d70c..bb0e3b0 100644 --- a/Assets/Scripts/PlayerController.cs +++ b/Assets/Scripts/PlayerController.cs @@ -34,20 +34,25 @@ private void Start() audioManager = GameObject.FindGameObjectWithTag("Audio").GetComponent(); } - // Update is called once per frame + // review(27.06.2024): Я бы вообще разбил этот метод на несколько MonoBehaviour, отвечающих за Dash/Movment/Jump соответственно private void Update() { if (isDashing || IsGameOver) return; + // review(27.06.2024): HandleDash() if ((Input.GetKeyDown(KeyCode.RightShift) || Input.GetKeyDown(KeyCode.LeftShift)) && canDash && penguinName == PenguinNames.Krico && !playerTrigger.isGrounded) StartCoroutine(Dash()); + + // review(27.06.2024): HandleMovement() var direction = Input.GetAxis("Horizontal"); isMoving = Math.Abs(direction) > 0.01; + // review(27.06.2024): Если не isMoving, то можно не менять позицию игрока, чтобы зря лишний раз движок не дергать transform.position += new Vector3(direction, 0, 0) * (speed * Time.deltaTime); + // review(27.06.2024): HandleJump() if (Input.GetKeyDown(KeyCode.Space)) { if (playerTrigger.isGrounded || doubleJump) diff --git a/Assets/Scripts/PlayerTrigger.cs b/Assets/Scripts/PlayerTrigger.cs index 1adfa34..b7c43b4 100644 --- a/Assets/Scripts/PlayerTrigger.cs +++ b/Assets/Scripts/PlayerTrigger.cs @@ -7,7 +7,7 @@ public class PlayerTrigger : MonoBehaviour public bool isGrounded; public Player otherPlayer; public bool isOnMovingPlatform; - public HashSet triggeredInteractableObjects = new(); + public HashSet triggeredInteractableObjects = new(); // review(27.06.2024): Разве GameObject можно использовать в Dictionary/HashSet? private MovingObject movingPlatform; private GameObject otherObject; diff --git a/Assets/Scripts/Text/PrinterSpecifics.cs b/Assets/Scripts/Text/PrinterSpecifics.cs index 5faea78..645c0d8 100644 --- a/Assets/Scripts/Text/PrinterSpecifics.cs +++ b/Assets/Scripts/Text/PrinterSpecifics.cs @@ -10,7 +10,7 @@ public class PinterSpecifics : MonoBehaviour [SerializeField] private float waitingTimeInterval = 0.065f; [SerializeField] private GameObject panel; private bool isTriggered; - public bool forKavazaki; + public bool forKavazaki; // review(26.06.2024): А что если вы решите добавить еще одного игрока? Снова forPlayer писать? Тут, кажется, имело смысл PlayerName[] supportedPlayers; поле ввести public bool forKrico; public bool forEstriper; public bool forCago; @@ -20,7 +20,7 @@ public class PinterSpecifics : MonoBehaviour public bool instantText; private bool isDialog; public bool multipleText; - private Collider2D PlayerCollider; + private Collider2D PlayerCollider; // review(26.06.2024): вам на самом деле нужен не коллайдер, а сам игрок, поэтому я бы оставил поле Player player; private void Update() { @@ -32,8 +32,8 @@ private void Update() || (penguinName == PenguinNames.Kawazaki && forKavazaki) || (penguinName == PenguinNames.Krico && forKrico) || (penguinName == PenguinNames.Estriper && forEstriper)) - && GameState.ChecksBool.TryGetValue(FlagName, out var dialogFlag) - && !GameState.IsNowTextDisplayed) + && GameState.ChecksBool.TryGetValue(FlagName, out var dialogFlag) // review(26.06.2024): спокойно заменяется на Contains + && !GameState.IsNowTextDisplayed) // review(27.06.2024): Это свойство используется только этим классом. Может, стоило сделать его просто полем класса? { textFrame.text = string.Empty; isDialog = true; @@ -79,6 +79,6 @@ private IEnumerator TypeLine() if (freezePlayer) GameState.ActivePlayer.isActive = true; GameState.IsNowTextDisplayed = false; - waitingTimeInterval = 0.065f; + waitingTimeInterval = 0.065f; // review(27.06.2024): магическая константа } } \ No newline at end of file diff --git a/Assets/Scripts/cameraFollowPlayer.cs b/Assets/Scripts/cameraFollowPlayer.cs index b3ef743..4a56fbf 100644 --- a/Assets/Scripts/cameraFollowPlayer.cs +++ b/Assets/Scripts/cameraFollowPlayer.cs @@ -15,11 +15,12 @@ void Start() .ToArray(); } + // review(27.06.2024): Лишние комменты можно удалить // Update is called once per frame void Update() { var temp = transform.position; - var player = players.FirstOrDefault(player => player.isActive); + var player = players.FirstOrDefault(player => player.isActive); // review(27.06.2024): можно было использовать GameState.ActivePlayer, наверное if (player is null) return; var position = player.transform.position;