A new way to change phase

[Turtyo]

Is your amelioration related to a problem? Please describe.

The way we change phase actually is to call the next phase by name when arriving at the end of the current phase. This makes it error prone because if we somehow write the wrong phase, tracking in the entire codebase where we called the wrong phase won't easy.

Describe the solution you'd like

Instead of calling the next phase by name, create a advance_to_next_phase function in the phase manager. At the end of the code of a phase, simply call this advance_to_next_phase function. This function changes phases in the order the phases are written in the Phase enum in GlobalEnums
Two advantages to this:

• no more error possible with calling the wrong phase (or if there is an error, it's in a single place)
• if later we want to change the order of the phases, it's simply a matter of rewriting a single enum (much simpler than to track down each phase change in the entire code)

Additional context

This might need to wait for #114 to be finished since we are discussing adding phases in there.
Additionally, set_phase would not be used directly anymore by other classes outside of the PhaseManager

[Turtyo]

Changing the priority because this will allow to fix #120

[Turtyo]

Checking the phase enum:

enum Phase
{
	NONE, ## Hello Youston, we are nowhere
	GAME_STARTING, ## The game is starting
	PLAYER_ATTACKING, ## The player is attacking
	PLAYER_FINISHING, ## The player finished its turn (card discard and other stuff starts here)
	ENEMY_ATTACKING, ## Enemy turn
	SCENE_END, ## The current scene ended
}

Not all phases make sense to go to with the advance_to_next_phase
I thus think there are two solutions:

• keep the enum as is, and when ending on NONE, GAME_STARTING or SCENE_END skip those
• change the enums. Make a GlobalPhase enum and a CombatPhase enum as follow:

enum GlobalPhase
{
	NONE, ## Hello Youston, we are nowhere
	GAME_STARTING, ## The game is starting
	SCENE_END, ## The current scene ended
}

I would also rename GAME_STARTING in SCENE_STARTED since it's actually what it does from what I can remember (up to the one implementing to check this claim 😆 )

enum CombatPhase
{
	PLAYER_ATTACKING, ## The player is attacking
	PLAYER_FINISHING, ## The player finished its turn (card discard and other stuff starts here)
	ENEMY_ATTACKING, ## Enemy turn
}

and the Phase manager next phase would be named advance_to_next_combat_phase

We would keep set_phase public to be used to go to SCENE_END for example, but it's fine to do it like that because it's a special case

[Turtyo]

misslick on that confirmed label. Anyway, taking this and i'm doing it as soon as it's confirmed and #114 has been merged

[Tomzkk]

I'm trying to think if there's any other way we could go about changing phases in general 🤔

Your idea, if I understand it correctly, will help us track where the phase changes are happening in a simpler way and prevent mistypes of phases, which would be great. But I feel at the same time it could actually make it even harder to figure out what is happening in which phase - the code will do it's magic and then just call a generic advance_to_next_phase - without giving us any visibility into where the current action is happening (at what point during combat, which phase etc)

It would work if we had a code for a "phase" in one place - like you say, at the end of a phase, go to next one. And currently, it would probably work, because most if not all phase changes are being handled in the battler if I remember correctly ? But I'm thinking if we add more phases, will it still handled in there ?

[Turtyo]

To answer your questions

• yes most of the phases are for combat, with the exception of game starting and event ended (and game starting might be better renamed as event started)
• this managing of the phases would probably only make sense in the battler, because it's always the same order
• if we add phases for other stuff than combat, then this probably wouldn't be handle by the next_phase

Now for the implementation I was thinking of this (pseudo-code)

advance_to_next_combat_phase():
    convert phase enum to an array of names
    check current phase
    go in the array until you find the current phase
    set the current phase to the one coming just after

or an alternative (which I prefer)

_ready():
    set a counter at 0
    set current phase as the first of the enum

advance_to_next_combat_phase():
    add 1 to the counter
    check that the counter is not more than the length of the phase enum
    if it's more, go back to 0
    now set the phase as the one of index "counter"

[Tomzkk]

Implemeting this as stated here should help the current state so I think we are good to go with it. My worries were mainly for the future should phases be added and mainly if they were to be controled/switched from various places. That should not really happen and if it does we can't really tell now how it would look so we should probably go with what helps us now and adjust later should it be needed.

Second implementation sounds more easily comprehensible to me

[Turtyo]

I'll go with the second implementation then, as it's the one I would have done too