Phase changes and passing priority are cryptic

[alexAG76]

First, some questions:

• Why does passing priority into the next phase not automatically call Phase.handleBeginPhase? It seems this is only called from InputControl.updateInput when Phase.doPhaseEffects returns true.
• Can someone explain why MagicStack.add calls Phase.passPriority when the simultaneousStackEntryList is non-empty?

Now I shall report some strangeness. This is from my game-state unit-tests, with my comments in dark red:

Game-state initialized.
Cards added to Human's library.
Human draws 5 cards.
Cards added to Computer's library.
Computer draws 3 cards.
Phase.handleBeginPhase() completed.
For Turn 1 (Human's) Upkeep Step, Human's possible moves are [passes priority].
Human passes priority.
For Turn 1 (Human's) Upkeep Step, Computer's possible moves are [passes priority].
So far, so good.
Computer passes priority.
For Turn 1 (Human's) Upkeep Step, Human's possible moves are [passes priority].
What? We should be in the Draw step now.
Human passes priority.
For Turn 1 (Human's) Draw Step, Computer's possible moves are [passes priority].
Computer passes priority.
For Turn 1 (Human's) Draw Step, Human's possible moves are [passes priority].
Human passes priority.
OK, both players passed priority, but the Human player should have had priority at the start of the Draw step.
For Turn 1 (Human's) Main1 Step, Computer's possible moves are [passes priority].
Computer passes priority.
For Turn 1 (Human's) Main1 Step, Human's possible moves are [plays land "Island", passes priority].
Human plays land "Island".
For Turn 1 (Human's) BeginCombat Step, Human's possible moves are [passes priority].
How did we get to BeginCombat from Main1 without both players passing? Granted, nobody had any other possible moves to make, so maybe this is an optimization. Am I right? It does not seem to be a case of auto-passing priority (e.g., from GUI settings).

I'm considering fixing this in the following manner:

• Augment Phase.passPriority so that it maintains a Set of the players who have passed when the stack was empty. Once all players have passed, it gives priority to the turn-owner and sets needToNextPhase.
• I'll try to retain the calls (which I do not comprehend*) to InputControl.resetInput, MagicStack.chooseOrderOfSimultaneousStackEntryAll, MagicStack.hasSimultaneousStackEntries, and MagicStack.resolveStack.
• Change MagicStack so that whenever something is pushed onto the MagicStack, it resets Phase's memory of what players have passed. (All players must pass priority for a phase to end.)

*Otherwise, I'm sure to introduce bugs.

[Sloth]

Once again this part of the code has grown from something really simple (I can remember when the AI only played cards during his Main 1 and lot of phases and steps were missing completely) and went through a lot of changes/fixes. The AI not beeing able to respond to the human playing lands (and maybe even casting spells) in main 1 is a serious disadvantage.

You seem to know what you're doing, so please by all means submit your changes.

[Chris H.]

Alex, you can send your google gmail address to Dennis Bergkamp via private message and he in turn can give you commit status to our SVN.

[Hellfish]

The SimultaneousStackEntry functions are for triggers going off simultaneously. chooseOrderof... presents a list of triggers to the human to order them onto the stack and lets the AI order it's triggers in APNAP order, if there are more than 1 triggers waiting,of course.

MagicStack.add passes priority if there are any Simultaneous trigger entries because I believe I added that as a bugfix to triggered abilities that look for spells cast not being added to the stack right away, but waiting until priority was passed otherwise, which was far too late. Phase.passPriority() calls the chooseOrderOf... method because that's ideally the only place where it should be called from, rules-compliance-wise.

EDIT: Also unrelated question because it will be bugging me otherwise: Is that not Gabriel from Constantine in your avatar?

[friarsol]

Some quick answers: All of Forge is based around this cryptic Input system. I think it was based around the original IDE Rares was using to get Forge up and running relatively quickly. At this point it has left us with leftover code that we need to work around so not break the system.

PassingPriority doesn't call straight into BeginPhase because of how this Input system works. Since I'm the one who most recently updated the Stack system, I recall that trying to call the BeginPhase directly led to multiple BeginPhase events happening (drawing two cards, dealing double damage, things like that), which is why UpdateInput also checks to make sure it actually is the beginning of a phase before calling there.

While I'm not saying your Unit Test is wrong (it would be great to have a test suite) I haven't noticed any issues with Priority being passed in my months of testing/playing since the Phase code was most recently rewritten. It seems like your Priority wasn't set properly when your Test first started which caused lots of issues down the road. Are you sure your Game State is correct when your Test starts?

• Augment Phase.passPriority so that it maintains a Set of the players who have passed when the stack was empty. Once all players have passed, it gives priority to the turn-owner and sets needToNextPhase.
• Change MagicStack so that whenever something is pushed onto the MagicStack, it resets Phase's memory of what players have passed. (All players must pass priority for a phase to end.)

passPriority already does exactly what you are suggesting you change it to. getPriorityPlayer() is determining which player has priority, and getFirstPriority() is determining who acted first in this segment of the Stack. If the current priority player is also the first player to act, Priority is given to the opponent. Otherwise, either the Stack Resolves or the Phase ends. Then updateInput() checks the game state and moves the game along.

[alexAG76]

Some quick answers: All of Forge is based around this cryptic Input system. ... PassingPriority doesn't call straight into BeginPhase because of how this Input system works. Since I'm the one who most recently updated the Stack system, I recall that trying to call the BeginPhase directly led to multiple BeginPhase events happening (drawing two cards, dealing double damage, things like that), which is why UpdateInput also checks to make sure it actually is the beginning of a phase before calling there.

Thank you; that is exactly the information I needed.

While I'm not saying your Unit Test is wrong (it would be great to have a test suite) I haven't noticed any issues with Priority being passed in my months of testing/playing since the Phase code was most recently rewritten.

Thanks, but I am now pretty sure the unit-test is wrong. It doesn't interact much with the InputControl or Input systems. I intend to change that. You are correct in that using the regular GUI works just fine with this. I had a nagging suspicion it had something to do with Input, because of the code references. I needed a mental push in that direction.

It seems like your Priority wasn't set properly when your Test first started which caused lots of issues down the road. Are you sure your Game State is correct when your Test starts?

No, because of the lack of interaction with InputControl and Input. I may have to trace through a regular initialization (of the GUI) to make sure I get it right.

• Augment Phase.passPriority so that it maintains a Set of the players who have passed when the stack was empty. Once all players have passed, it gives priority to the turn-owner and sets needToNextPhase.
• Change MagicStack so that whenever something is pushed onto the MagicStack, it resets Phase's memory of what players have passed. (All players must pass priority for a phase to end.)

passPriority already does exactly what you are suggesting you change it to. getPriorityPlayer() is determining which player has priority, and getFirstPriority() is determining who acted first in this segment of the Stack. If the current priority player is also the first player to act, Priority is given to the opponent. Otherwise, either the Stack Resolves or the Phase ends. Then updateInput() checks the game state and moves the game along.

OK; that was not obvious from reading the code. Thanks for the clarification.

I'm glad I listened to the voice in my head that said, "You need more help."

[Jaedayr]

I'm glad I listened to the voice in my head that said, "You need more help."

You only have one voice in your head that says that to you?! ALL of them in mine say that.