-
Notifications
You must be signed in to change notification settings - Fork 263
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
916 #917
916 #917
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice contribution!
@@ -300,7 +300,7 @@ def strategy(self, opponent: Player) -> Action: | |||
return C | |||
# TFT on round 2 | |||
if len(self.history) == 1: | |||
return D if opponent.history[-1:] == [D] else C |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we use this because that's how TFT is actually implemented in the library, where we don't know that the history length is greater than zero. I don't have a preference here and I think your line might be more readable, but we might prefer consistency. So I'm approving and I'll let @drvinceknight and @meatballs weigh in if they have a preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I prefer this too. This is how TFT was implemented a while ago but no longer :)
@@ -300,7 +300,7 @@ def strategy(self, opponent: Player) -> Action: | |||
return C | |||
# TFT on round 2 | |||
if len(self.history) == 1: | |||
return D if opponent.history[-1:] == [D] else C |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I prefer this too. This is how TFT was implemented a while ago but no longer :)
Closes #917 (so that github will close the issue for us). |
Issue #916