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

fix the comments for check_for_winner #1148

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

helpingstar
Copy link
Contributor

@helpingstar helpingstar commented Dec 21, 2023

Description

To determine the winner, tic tac toe uses check_for_winner(), defined in tictactoe/board.py. That function describes its returns as -1, 0, 1 in the comments as follows. However, the check_for_winner() function returns -1, 1, 2.

# returns:
# -1 for no winner
# 0 -- agent 0 wins
# 1 -- agent 1 wins
def check_for_winner(self):
winner = -1
for combination in self.winning_combinations:
states = []
for index in combination:
states.append(self.squares[index])
if all(x == 1 for x in states):
winner = 1
if all(x == 2 for x in states):
winner = 2
return winner

Other functions that use the check_for_winner() function also run the function thinking that the integers it returns are -1, 1, and 2. This leads us to believe that we should change the function's comment.

def check_game_over(self):
winner = self.check_for_winner()
if winner == -1 and all(square in [1, 2] for square in self.squares):
# tie
return True
elif winner in [1, 2]:
return True
else:
return False

if self.board.check_game_over():
winner = self.board.check_for_winner()
if winner == -1:
# tie
pass
elif winner == 1:
# agent 0 won
self.rewards[self.agents[0]] += 1
self.rewards[self.agents[1]] -= 1
else:
# agent 1 won
self.rewards[self.agents[1]] += 1
self.rewards[self.agents[0]] -= 1

The observe function also shows a correspondence of 1 -> agent0, 2 -> agent1.

# empty -- 0
# player 0 -- 1
# player 1 -- 2

Type of change

  • fix comment

Screenshots

before
image

after

image

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have run pytest -v and no errors are present.
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@elliottower
Copy link
Contributor

Thanks for catching this, looks good. The original code there isn't the cleanest by any means, could probably be made a bit less confusing, but it does what it's supposed to and isn't worth changing around IMO, so best to just make sure the comments reflect the current behavior

@elliottower elliottower merged commit cdbecd7 into Farama-Foundation:master Jan 11, 2024
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants