Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Assets/Scripts/BackGroundLoop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ private void Start()

private IEnumerator PlayBackgroundSounds()
{
while (true)
while (true) // review(30.06.2024): Не хватает способа выключить эти звуки, бесконечный цикл - не лучшая идея, стоило добавить хотя бы флажок какой-нибудь
{
foreach (var sound in backgroundSounds)
{
Expand Down
5 changes: 5 additions & 0 deletions Assets/Scripts/CameraController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ private void FindPlayer(bool playerIsLeft)
{
player = GameObject.FindGameObjectWithTag("CameraView").transform;
lastX = Mathf.RoundToInt(player.position.x);

// review(30.06.2024): Можно еще так было, кмк более читаемо
var direction = playerIsLeft ? Vector3.left : Vector3.right;
transform.position = player.position + direction * offset.x;

transform.position = playerIsLeft
? new Vector3(player.position.x - offset.x, player.position.y,
Expand All @@ -50,6 +54,7 @@ private void Update()
isLeft = true;
lastX = Mathf.RoundToInt(player.position.x);

// review(30.06.2024): Дублируется логика
var target = isLeft
? new Vector3(player.position.x - offset.x, player.position.y,
transform.position.z)
Expand Down
8 changes: 7 additions & 1 deletion Assets/Scripts/CasinoTrigger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ private void Update()
{
return;
}


// review(30.06.2024): Зачем каждый раз проверять E? Может, стоило вынести метод типа if (E pressed) HandleCasinoActivating();
if (Input.GetKeyDown(KeyCode.E) && Inventory.Feather == 0 && meetingCount == 2 && PlayerController.Balance == 1)
{
if (!GoAway.isPlaying)
Expand All @@ -47,6 +48,7 @@ private void Update()

if (Input.GetKeyDown(KeyCode.E) && ((Inventory.Feather == 0 && meetingCount == 2) || PlayerController.Balance == 0))
{
// review(30.06.2024): Хорошо бы тут использовать метод AnySoundPlaying()
if (TicketSound.isPlaying || DKRSound.isPlaying || MorsyankaSound.isPlaying
|| GoAway.isPlaying || casinoSound.isPlaying) return;
ErrorSound.Play();
Expand All @@ -55,6 +57,7 @@ private void Update()

if (Input.GetKeyDown(KeyCode.E) && PlayerController.Balance != 0 && meetingCount <= 2)
{
// review(30.06.2024): Проверка дублируется
if (TicketSound.isPlaying || DKRSound.isPlaying || MorsyankaSound.isPlaying
|| GoAway.isPlaying || casinoSound.isPlaying) return;
casinoSound.Play();
Expand All @@ -67,6 +70,7 @@ private void Update()

private IEnumerator StartTimer()
{
// review(30.06.2024): Эта логика по прежнему дублируется
isCounting = true;
while (timeToCount > 0)
{
Expand All @@ -79,6 +83,7 @@ private IEnumerator StartTimer()
timeToCount = 2f;
switch (meetingCount)
{
// review(30.06.2024): Интересно, почему в обратном порядке?
case 2:
Inventory.Morsynka = 1;
MorsyankaSound.Play();
Expand All @@ -93,6 +98,7 @@ private IEnumerator StartTimer()
break;
}

// review(30.06.2024): Я бы изменил этот счетчик на enum CasinoStage, т.к. по замыслу это как будто ближе, да и более понятно, чем какое-то число
meetingCount += 1;
}

Expand Down
7 changes: 4 additions & 3 deletions Assets/Scripts/ChessTrigger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
public class ChessTrigger : MonoBehaviour
{
private bool isPlayerNear;
private static int win;
private static int isPlay;
private static int win; // review(30.06.2024): bool?
private static int isPlay; // review(30.06.2024): Почему статичекое поле и почему не bool?
public GameObject balance;
public GameObject board;
public GameObject boardOnLavka;
Expand All @@ -20,7 +20,7 @@ public class ChessTrigger : MonoBehaviour
public AudioSource audioSource;
public AudioClip buttonClickSound;

public int gameState;
public int gameState; // review(30.06.2024): enum GameState, пусть даже приватный
public float timeToCount;
private bool isCounting;

Expand Down Expand Up @@ -109,6 +109,7 @@ IEnumerator StartTimer()
isPlay = 0;
}

// review(30.06.2024): Тут как будто не хватает проверки на то, что это действительно игрок столкнулся
private void OnTriggerEnter2D(Collider2D other) => isPlayerNear = true;

private void OnTriggerExit2D(Collider2D other) => isPlayerNear = false;
Expand Down
6 changes: 5 additions & 1 deletion Assets/Scripts/GooseTrigger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ private void Start()
{
meetingCount = 0;

// review(30.06.2024): Код дублируется. Мне кажется, было бы уместно создать extension типа public static Color WithAlpha(this Color color, float alpha);
Color color = feather.color;
color.a = 0f;
feather.color = color;
Expand Down Expand Up @@ -86,6 +87,7 @@ private void Update()
if (meetingCount == 2 && IsDKRInInventory)
{
IsDKRInInventory = false;
// review(30.06.2024): Дублирование кода из Start()
var color = dkr.color;
color.a = 0f;
dkr.color = color;
Expand All @@ -112,7 +114,7 @@ private void ShowItem()
dkrView.enabled = true;
IsDKRInInventory = true;
StartCoroutine(FadeInDKR());
GetComponent<AudioSource>().Play();
GetComponent<AudioSource>().Play(); // review(30.06.2024): Наверное, имело смысл запросить этот компонент в Start()
}

if (Inventory.Ticket == 1)
Expand All @@ -133,6 +135,7 @@ private void ShowItem()
}
}

// review(30.06.2024): Почти все FadeIn/Out дублируются. Стоило выделить один общий метод, а не дублировать так много
IEnumerator FadeInDKR()
{
dkrView.color = new Color(dkrView.color.r, dkrView.color.g, dkrView.color.b, 0);
Expand Down Expand Up @@ -196,6 +199,7 @@ IEnumerator FadeInMorsyanka()

while (MorsynkaView.color.a < 1)
{
// review(30.06.2024): Вот тут тоже бы пригодился extension WithAlpha
MorsynkaView.color = new Color(MorsynkaView.color.r, MorsynkaView.color.g, MorsynkaView.color.b, MorsynkaView.color.a + Time.deltaTime);
yield return null;
}
Expand Down
28 changes: 27 additions & 1 deletion Assets/Scripts/Inventory.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System.Collections.Generic;
using UnityEngine;

public class Inventory : MonoBehaviour
public class Inventory : MonoBehaviour // review(30.06.2024): Зачем наследование от монобеха?
{
// review(30.06.2024): Мне кажется, что стоило инвентарную систему сделать чуть по-другому, т.к. у вас все предметы подразумеваются в единственном экземпляре, то лучше было бы использовать enum
public static int Ticket;
public static int Dkr;
public static int Ship;
Expand All @@ -19,3 +21,27 @@ private void Start()
Key = 0;
}
}

// review(30.06.2024): Я говорю про что-то такое
public static class Inventory_Refactored
{
private static readonly HashSet<Item> Items = new();

public static bool Contains(Item item) => Items.Contains(item);

public static void Add(Item item) => Items.Add(item);

public static void Remove(Item item) => Items.Remove(item);

public static IEnumerable<Item> GetAll() => Items;
}

public enum Item
{
Ticket,
Dkr,
Ship,
Morsynka,
Feather,
Key,
}
1 change: 1 addition & 0 deletions Assets/Scripts/LiftButtonTrigger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class LiftButtonTrig : MonoBehaviour

IEnumerator StartTimer()
{
// review(30.06.2024): Очередное дублирование логики таймера
isCounting = true;
while (timeToCount > 0 && isCounting)
{
Expand Down
1 change: 1 addition & 0 deletions Assets/Scripts/LiftController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public class LiftController : MonoBehaviour

IEnumerator StartTimer()
{
// review(30.06.2024): Слишком много комментариев. Они все не нужны
isCounting = true; // Устанавливаем флаг, что таймер запущен
while (timeToCount > 0) // Пока время не истечет
{
Expand Down
1 change: 1 addition & 0 deletions Assets/Scripts/Manager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public void Play()

private IEnumerator FadeIn()
{
// review(30.06.2024): Дублирование FadeIn-а
var delta = 0.0f;
while (delta < 1)
{
Expand Down
1 change: 1 addition & 0 deletions Assets/Scripts/OldLiftButtonTrigger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using UnityEngine;
using UnityEngine.SceneManagement;

// review(30.06.2024): Если он старый, то почему используется?
public class OldLiftButtonTrigger : MonoBehaviour
{
public GameObject otherObject;
Expand Down
1 change: 1 addition & 0 deletions Assets/Scripts/PlayerController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ private void Update()
AdditionalTimer();
}

// review(30.06.2024): Непонятно, что такое Quantity, quantity чего?
private void UpdateQuantity()
{
if (quantity != null)
Expand Down
2 changes: 1 addition & 1 deletion Assets/Scripts/Rail trigger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ private void Update()
Player.SetActive(false);
GetComponent<AudioSource>().Play();
StartCoroutine(StartTimer());
Player.transform.position = new Vector3(980, upY, -2);
Player.transform.position = new Vector3(980, upY, -2); // review(30.06.2024): Почему захардкожены константы? Может, стоило вынести весь вектор в поле?
}
if ((Input.GetKeyDown(KeyCode.S) || Input.GetKeyDown(KeyCode.DownArrow)) && !isLowestFloor)
{
Expand Down
1 change: 1 addition & 0 deletions Assets/Scripts/Sounds/GetVolumeByDistance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ private void HandleSpeech(float distance)

private void HandleBackgroundSound(float distance)
{
// review(30.06.2024): Проверки дублируют те, что выше
if (audioSource.isPlaying)
return;

Expand Down
1 change: 1 addition & 0 deletions Assets/Scripts/StartHintFadeOut.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ private IEnumerator WaitAndStartFadeOut(float waitTime)

private IEnumerator FadeOutAnimation()
{
// review(30.06.2024): var
float elapsedTime = 0.0f;
Color initialColor = spriteRenderer.color;

Expand Down