jak można zapiać prościej ten fragment kodu?

Wydaje mi się że chyba trochę tutaj namieszałem, bardzo trudno się połapać w tym fragmencie kodu, nie wiem czy to przez to jak zbudowałem te masę ifów czy powinienem rozwiązać ten problem w inny sposób. Generalnie jest to fragment stanu przeciwnika w grze IDLE, i przejścia ze stanu IDLE do innych stanów opierają się też na stanie jaki byl poprzednio:

        if (!isExitingState)
        {
            if (stateMachine.previousState == enemy.meleeAttackState)
            {
                if (!isPlayerDetected)
                {
                    if (isIdleTimeOver)
                    {
                        if (!isDectectingLedge && !flipAfterIdle)
                        {
                            core.Movement.Flip();
                        }

                        stateMachine.ChangeState(enemy.moveState);
                    }
                }
                else
                {
                    if (isIdleTimeOver)
                    {
                        if (isEnemyInRangeDetected && isIdleTimeOver)
                        {
                            stateMachine.ChangeState(enemy.meleeAttackState);
                        }
                        else if (isDectectingLedge && !isEnemyInRangeDetected)
                        {
                            stateMachine.ChangeState(enemy.chargeState);
                        }
                    }
                }
            }
            else
            {
                if (!isPlayerDetected)
                {
                    if (isIdleTimeOver)
                    {
                        if (!isDectectingLedge && !flipAfterIdle)
                        {
                            core.Movement.Flip();
                        }

                        stateMachine.ChangeState(enemy.moveState);
                    }
                }
                else
                {
                    if (isEnemyInRangeDetected && isIdleTimeOver)
                    {
                        stateMachine.ChangeState(enemy.meleeAttackState);
                    }
                    else if (isPlayerDetected && isDectectingLedge && !isEnemyInRangeDetected)
                    {
                        stateMachine.ChangeState(enemy.chargeState);
                    }
                }
            }
        }

Podziel na mniejsze metody (np. zawartość każdego if/else), a wyrażenia logiczne, które teraz masz bezpośrednio w ifach też wsadź do jakichś ładnie nazwanych metod. To powinno pomóc pozbyć się duplikacji kodu, no i będzie się dało zrozumieć co ma się kiedy wydarzyć.

Poza tym, co @somekind radzi, to może coś takiego? Przynajmniej nie ma drabinki ifów :D

var shouldMove = !isPlayerDetected && isIdleTimeOver;

var shouldFlip = shouldMove && (!isDectectingLedge && !flipAfterIdle);

if (shouldFlip)
{
    core.Movement.Flip();
}

if (shouldMove)
{
  stateMachine.ChangeState(enemy.moveState);
}

Masz tu trochę powtórzonego kodu. Wydziel go do metod i wykorzystaj ponownie. Widze tutaj też kombinację kilku bitów i kilku decyzji, możesz sobie to rozrysować w tabelce i spróbować zminimalizować, być może kod się bardzo uprości, być może nie, ale możesz sprawdzić.
Kod będący warunkami również wydziel do metod, a metody dobrze ponazywaj.
enemy wygląda jak enum, jeśli tak jest, to nazwij go oraz jego pola jak enum.
Podany kod odpowiada za strategię kroków w grze, zatem przemyśl przepisanie klasy do wzorca projektowego strategia, czyli metoda na podstawie pewnych warunków będzie zwracała następny stan.

Albo jeszcze inaczej. Trzeba spojrzeć wysokopoziomowo na ten kod i zrozumieć, co tak naprawdę chcemy zrobić. A chcemy:

1. Na podstawie jakichś warunków wywołać (albo nie) ChangeState z argumentem zależnym od warunków
2. Wywołać (albo i nie) Flip

Więc najprościej to tak:

1. Metoda State? GetNextState zwracająca następny stan albo od biedy nulla, jeśli stan ma się nie zmienić
2. Metoda bool ShouldFlip zwracająca, czy chcemy wykonać flipa, czy nie.

Kwestie wydajnościowe pomijamy :)

Fail fast. (lines 1-4)

Ale również trzeba pamiętać że nie da się w nieskończoność upraszczać kodu oraz nie powinno się wyciągać 50iątej metody nakładki

złożoność gdzieś musi być, te ifki nie znikną, a dodawanie kolejnych warstw może mieć przeciwny skutek do zamierzonego.

zamieniłbym if !a else na if a else

Wydaje mi się że nigdzie się nie walnąłem upraszczając niektóre warunki typu

if (isIdleTimeOver)
{
    if (isEnemyInRangeDetected && isIdleTimeOver)

A zatem:

if (isExitingState)
{
	return;
}
	
if (stateMachine.previousState == enemy.meleeAttackState)
{
	HandleMeleeAttack();
}
else
{
	HandleDefault();
}


HandleMeleeAttack()
{
	if (isIdleTimeOver)
	{
		if (isPlayerDetected)
		{
			if (isEnemyInRangeDetected)
			{
				stateMachine.ChangeState(enemy.meleeAttackState);
			}
			else if (isDectectingLedge)
			{
				stateMachine.ChangeState(enemy.chargeState);
			}
		}
		else
		{
			if (!isDectectingLedge && !flipAfterIdle)
			{
				core.Movement.Flip();
			}

			stateMachine.ChangeState(enemy.moveState);
		}
	}
}

HandleDefault()
{
	if (isPlayerDetected)
	{
		if (isEnemyInRangeDetected && isIdleTimeOver)
		{
			stateMachine.ChangeState(enemy.meleeAttackState);
		}
		else if (isPlayerDetected && isDectectingLedge && !isEnemyInRangeDetected)
		{
			stateMachine.ChangeState(enemy.chargeState);
		}
	}
	else if (isIdleTimeOver)
	{
		if (!isDectectingLedge && !flipAfterIdle)
		{
			core.Movement.Flip();
		}

		stateMachine.ChangeState(enemy.moveState);
	}
}

Patrząc na tą metodę wydaje mi się że z czasem będzie on większa i większa, więcej stanów i logiki. Ten kawałek kodu będzie szybko degradował.

Dlatego wydzieliłbym to do osobnych klas. To stan powinien być odpowiedzialny za to czy jest aktywny czy nie. Taki kod łatwiej testować.

Wyglądało by to mniej więcej tak:

        public interface State
        {
            bool IsActive();
            ...
        }

        public class MeleeAttackState : State
        {

            public bool IsActive()
            {
                if (stateMachine.previousState == enemy.meleeAttackState)
                {
                    if (!isPlayerDetected)
                    {
                        if (isIdleTimeOver)
                        {
                            if (!isDectectingLedge && !flipAfterIdle)
                            {
                                core.Movement.Flip();
                            }

                            return true;
                        }
                    }
                }

                return false;
            }
        }

Ok to powiem wam jak ja to rozwiązałem.

Problem:
Jest klasa MOVE_STATE w której przeciwnik patroluje i jak dojdzie do ściany czy przepaści przechodzi od IDLE_STATE na losową ilość czasu. Po dodaniu stanu MELEE_ATTACK_STATE pojawił się problem bo chciałem dodać delay pomiędzy atakami, więc początkowo chciałem dodać stan REST_STATE ale uznałem że wiele rzeczy bym kopiował z IDLE, łącznie z animacją, a różnicą jest tylko ilość czasu pozostania w IDLE. W praktyce utworzyła się drabinka ifów jak widzicie w moim pierwszym poście.

Konkluzja:
Przemyślałem jeszcze raz moją początkową myśl o REST_STATE, i w praktyce (w moim kodzie jest dużo dziedziczenia którego nie widzicie, używam FSM i każdy przeciwnik będzie dziedziczył od np. IDLE_STATE i miał swój odpowiednik) jak utworzę innego przeciwnika, który np. będzie po każdym ataku musiał wyciągać topór z ziemi, to przyda się tutaj inna animacja niż IDLE ANIMATION, a w obecnej konstrukcji jeszcze bardziej zagmatwałoby to sprawę, bo musiałbym dodawać warunki wyświetlania konkretnych animacji w tych bloczkach bądź klasie stanu, a każdy stan ma przypisaną dla przejrzystości jedną animację (tutaj wyjątkiem jest aktualnie gracz i stan ataku gdzie są różne combosy).

Rozwiązanie:
Utworzyłem więc klasę stanu AFTER_ATTACK_STATE i wygląda to zdecydowanie lepiej, w każdej chwili mogę dodać do przeciwnika inną animację tego co się dzieje po ataku, np. jest zmęczony i ma opuszczone ręce, czy wyciąga topór z ziemi, no i wszystko jest bardziej przejrzyste, oto kod jak wygląda teraz:

IDLE_STATE ten który widzieliście w pierwszym poście z drabinką ifów wygląda teraz tak:

    public override void LogicUpdate()
    {
        base.LogicUpdate();

        if (isIdleTimeOver)
        {
            if (!isPlayerDetected)
            {
                stateMachine.ChangeState(enemy.moveState);
            }
        }

        if (isEnemyInRangeDetected)
        {
            stateMachine.ChangeState(enemy.meleeAttackState);
        }
        else if (isPlayerDetected && isDectectingLedge && !isEnemyInRangeDetected)
        {
            stateMachine.ChangeState(enemy.chargeState);
        }
    }

A to nowy stan AFTER_ATTACK_STATE:

    public override void LogicUpdate()
    {
        base.LogicUpdate();

        if (isStateTimeOver)
        {
            if(isEnemyInRangeDetected)
            {
                stateMachine.ChangeState(enemy.meleeAttackState);
            }
            else if (isPlayerDetected && isDectectingLedge && !isEnemyInRangeDetected)
            {
                stateMachine.ChangeState(enemy.chargeState);
            }
            else if (!isPlayerDetected)
            {
                stateMachine.ChangeState(enemy.moveState);
            }
        }
    }

@Wilktar:

Taki kod łatwiej testować.

w obu przypadkach powinno być łatwo gdyby zastosować to, o czym pisał @nobody01 oraz @Saalin

ale pomysł ciekawy

jak można zapiać prościej ten fragment kodu?

O tak: kukuryku!

A na poważnie, to mógłbyś narysować graf stanów/przejść między stanami i tu wkleić?

Łatwiej byłoby się zorientować w logice, jeśli byłoby to narysowane. I łatwiej też wykryć związki między stanami / uprościć.

            if(isEnemyInRangeDetected)
            {
                stateMachine.ChangeState(enemy.meleeAttackState);
            }
            else if (isPlayerDetected && isDectectingLedge && !isEnemyInRangeDetected)

Tu masz tautologię z tym !isEnemyInRangeDetected, bo jeśli wchodzi w blok else, to wiadomo, że isEnemyInRangeDetected będzie fałszywe, bo gdyby było prawdziwe, to weszłoby w pierwszy blok.

LukeJL napisał(a):

jak można zapiać prościej ten fragment kodu?

O tak: kukuryku!

A na poważnie, to mógłbyś narysować graf stanów/przejść między stanami i tu wkleić?

Łatwiej byłoby się zorientować w logice, jeśli byłoby to narysowane. I łatwiej też wykryć związki między stanami / uprościć.

            if(isEnemyInRangeDetected)
            {
                stateMachine.ChangeState(enemy.meleeAttackState);
            }
            else if (isPlayerDetected && isDectectingLedge && !isEnemyInRangeDetected)

Tu masz tautologię z tym !isEnemyInRangeDetected, bo jeśli wchodzi w blok else, to wiadomo, że isEnemyInRangeDetected będzie fałszywe, bo gdyby było prawdziwe, to weszłoby w pierwszy blok.

Schematy blokowe to moja słaba strona, nigdy nie robię z lenistwa, muszę to zmienić jak najszybciej. Mam za to ten z Unity:

I na jakiej podstawie jest podejmowana decyzja o przejściu z jednego stanu do drugiego? Tego w sumie brakuje w tym diagramie.
Z drugiej strony ja bardziej bym myślał nie w kategoriach ifów i sprawdzania flag, tylko bardziej w kategoriach zdarzeń (stan idle -> zdarzenie enemyInRangeDetected -> więc przechodzimy do stanu attack). I w sumie idąc w tę stronę można iść w tabelkę przejść: https://en.wikipedia.org/wiki/State-transition_table
https://en.wikipedia.org/wiki/Finite-state_machine

if (isIdleTimeOver)

To jest problematyczna flaga. Tak często tego używasz, że wydaje się, że to powinno być oddzielnym stanem (może IDLE -> READY, gdzie nowy stan READY byłby stanem, który następuje po IDLE, wtedy jak "idle time" będzie over?).

coś takiego (jeśli dobrze rozumiem twój zamysł)

LukeJL napisał(a):

I na jakiej podstawie jest podejmowana decyzja o przejściu z jednego stanu do drugiego? Tego w sumie brakuje w tym diagramie.
Z drugiej strony ja bardziej bym myślał nie w kategoriach ifów i sprawdzania flag, tylko bardziej w kategoriach zdarzeń (stan idle -> zdarzenie enemyInRangeDetected -> więc przechodzimy do stanu attack). I w sumie idąc w tę stronę można iść w tabelkę przejść: https://en.wikipedia.org/wiki/State-transition_table
https://en.wikipedia.org/wiki/Finite-state_machine

if (isIdleTimeOver)

To jest problematyczna flaga. Tak często tego używasz, że wydaje się, że to powinno być oddzielnym stanem (może IDLE -> READY, gdzie nowy stan READY byłby stanem, który następuje po IDLE, wtedy jak "idle time" będzie over?).

coś takiego (jeśli dobrze rozumiem twój zamysł)

tak, rozwiązałem to osobnym stanem, nie wiem czy widziałeś pare postów wyżej.