Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Game.endTurn() is bugged. #463

Closed
LevelX2 opened this issue Jul 17, 2014 · 7 comments
Closed

Game.endTurn() is bugged. #463

LevelX2 opened this issue Jul 17, 2014 · 7 comments
Assignees

Comments

@LevelX2
Copy link
Contributor

@LevelX2 LevelX2 commented Jul 17, 2014

From emerald000:
Game.endTurn() is currently bugged, and EndTurnEffect is by extension. Time Stop and Sundial of the Infinite aren't doing anything and Time Vault is quite better as you don't skip your turn to untap it.

@LevelX2
Copy link
Contributor Author

@LevelX2 LevelX2 commented Jul 17, 2014

I've seen that the game.endTurn(playerId) only ends the turn of the currently active player. It's not clear to me, why this check is done. Spontaneous i would say this check can be removed.

The EndTurnEffect sets the ability controller as value for the playerId. So I guess this logic is not ok. Is this maybe also the cause of the problems you found?

Or did you find also any problems, if the active player ends it's turn?

View commit the endTurn method was added:
a00701c

@LevelX2
Copy link
Contributor Author

@LevelX2 LevelX2 commented Jul 17, 2014

Maybe @magenoxx knows why the check for the active player id is/was neccessary.
To me it seems that it can be removed.

@emerald000
Copy link
Contributor

@emerald000 emerald000 commented Jul 17, 2014

I guess it is to prevent skipping another player's turn if somehow the turn changed while the ability was on the stack. Not sure how that could happen, but I guess that was the reasoning.

And no matter who casts Time Stop when, the turn actually doesn't end. It will actually go through the steps outlined in Turn.endTurn(), but won't actually end the turn. The game will carry on in whatever step it was.

@LevelX2
Copy link
Contributor Author

@LevelX2 LevelX2 commented Jul 17, 2014

I guess it is to prevent skipping another player's turn if somehow the turn changed while the ability was on the stack. Not sure how that could happen, but I guess that was the reasoning.

But why is the controllerId set to the parameter? If you would be right, the active player id has to be set to the parameter.

@LevelX2
Copy link
Contributor Author

@LevelX2 LevelX2 commented Jul 17, 2014

To end the turn this code in Turn.endTurn() should do it.

   Phase phase = new EndPhase();
   phase.setStep(new CleanupStep());
   currentPhase = phase;

So we have to check, why it doesn't work.

@emerald000
Copy link
Contributor

@emerald000 emerald000 commented Jul 17, 2014

You are right. I was not thinking that it should be activePlayerId. I have no idea then; might be a bug.

@LevelX2 LevelX2 self-assigned this Jul 17, 2014
@LevelX2
Copy link
Contributor Author

@LevelX2 LevelX2 commented Jul 17, 2014

I guess the phases go on because later some code in Turn.play was commented out, that stopped the phases loop before.

// magenoxx: this causes bugs when we need to add several phases connected with each other (WorldAtWarTest)
            //if (!currentPhase.equals(phase)) // phase was changed from the card
              //  break;

I'll look into this and will try to fix it.

@LevelX2 LevelX2 closed this in 0209580 Jul 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants