Skip to content

Couple of fixes#14

Open
VinQbator wants to merge 19 commits intowenkesj:masterfrom
VinQbator:master
Open

Couple of fixes#14
VinQbator wants to merge 19 commits intowenkesj:masterfrom
VinQbator:master

Conversation

@VinQbator
Copy link

Fixed #12 and #13

Copy link
Contributor

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments on what I've seen thus far. The commit history is a little bit messy - in the future I'd recommend self-contained, atomic commits, ideally with messages that describe what and why something has been done.

# -*- coding: utf-8 -*-
#
# Copyright (c) 2018 Sam Wenke (samwenke@gmail.com)
# Copyright (c) 2019 Ingvar Lond (ingvar.lond@gmail.com)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine by me to add a copyright note, but it's less fine to sneak it in a "bugfix" commit. Commit messages should ideally reflect all the changes made, even if they're "bureaucrat" ones.

However, at this point I'd like to defer to @wenkesj as to whether he's fine with this addition, as I don't feel comfortable merging this in.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that. Doesn't have to be there if you don't like it.

Copy link

@BigBadBurrow BigBadBurrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the player folds, the self._current_player is not moved on (self._current_player = self._next(players, self._current_player)). So the same player is again asked for an action, which again is fold, and now when it tries to remove the player there's a ValueError as the player was already removed (see trace below).

The player needs to be moved on before removing from players:

      if move[0] == 'fold':
        self._current_player.playing_hand = False
        folded_player = self._current_player
        self._current_player = self._next(players, self._current_player)
        players.remove(folded_player)
        self._folded_players.append(folded_player)
      else:      
        self._current_player = self._next(players, self._current_player)

total pot: 39460
last action by player 3:
x fold
community:

  • [J♦],[8♠],[T♦],[3♦],[9♦]
    players:
    0 [6♦],[2♣] stack: 180
    1 SB [6♣],[J♥] stack: 126
    2 BB [7♥],[K♥] stack: 54
    3 [6♥],[5♠] stack: 180
    Getting move from agent for player 3 (Agent: 4)
    Player 3 Move: [3,0]
    Player 3 ('fold', -1)
    Traceback (most recent call last):
    [...]
    File "c:\Development\holdem\env.py", line 206, in step
    players.remove(folded_player)
    ValueError: list.remove(x): x not in list

@VinQbator
Copy link
Author

@BigBadBurrow I suspect you are not resetting the environment after getting done==True

@VinQbator
Copy link
Author

@BigBadBurrow can you test if my fork works with your setup?

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.

3 participants