Bet changes

[amortization]

Continuation of #18 and discussion in #13.

I changed Bet to add another layer of abstraction separating Bets that use winning_numbers and losing_numbers from those that don't (currently just HardWay and Fire) and making those a new type of abstract Bet called WinningLosingNumbersBet. I pushed the _update_bet logic from Bet to WinningLosingNumbersBet as it relied on winning and losing numbers.

WinningLosingNumbersBet also makes the winning_numbers and losing_numbers attributes required for any class that inherits from it since all classes that inherit from it should use it in some fashion.

I also made name as a property of Bet equal to class.name. This allowed me to remove all of the unecessary init methods since everything being set at initiation was controlled by the Base class. For Example

class Place4(Place):
    def __init__(self, bet_amount: float):
        super().__init__(bet_amount)
        self.name: str = "Place4"
        self.winning_numbers: list[int] = [4]
        self.losing_numbers: list[int] = [7]
        self.payoutratio: float = 9 / 5

Became:

class Place4(Place):
    payout_ratio: float = 9 / 5
    winning_numbers: list[int] = [4]
    losing_numbers: list[int] = [7]

It also let me remove a number of _update_bet methods and simplify them so something like Field went from:

class Field(Bet):
    def __init__(self, bet_amount: float):
        super().__init__(bet_amount, None)
        self.name: str = "Field"
        self.winning_numbers: list[int] = [2, 3, 4, 9, 10, 11, 12]
        self.losing_numbers: list[int] = [5, 6, 7, 8]

    @property
    def payout_ratio(self):
        return self.table.payouts['field_ratios'][self.table.dice.total]

    def _update_bet(self) -> tuple[str | None, float, bool]:
        win_amount: float = 0.0
        remove: bool = True

        if self.table.dice.total in self.winning_numbers:
            status = "win"
            win_amount = self.payout_ratio * self.bet_amount
        else:
            status = "lose"

        return status, win_amount, remove

to this:

class Field(WinningLosingNumbersBet):
    @property
    def payout_ratio(self):
        return self.table.payouts['field_payouts'][self.table.dice.total]

    @property
    def winning_numbers(self):
        return list(self.table.payouts['field_payouts'])

    @property
    def losing_numbers(self):
        return list(x for x in [2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12] if x not in self.table.payouts['field_payouts'])

[amortization]

I made status, win_amount and remove properties of bet. This simplified a lot of repeating logic used in _update_bet and allows update_bet to actually update the bet given the table status instead of having to return something. This would allow end users to just look at bet properties instead of having to call update_bet to get those properties.

[amortization]

@skent259 how do you feel about splitting up Odds bets into Odds4, Odds5 etc? This would apply to LayOdds as well. I think that there would be a lot of benefits that would go along with that:

• Remove subname requirement
• Easier for the users for player.bet(Odds6(6)) as opposed to having get the PassLine or Come bet object to pass in the Odds.
• Bet.allowed() can be used to determine whether the player has the required PassLine and Come bets.

[skent259]

@skent259 how do you feel about splitting up Odds bets into Odds4, Odds5 etc? This would apply to LayOdds as well. I think that there would be a lot of benefits that would go along with that:

• Remove subname requirement
• Easier for the users for player.bet(Odds6(6)) as opposed to having get the PassLine or Come bet object to pass in the Odds.
• Bet.allowed() can be used to determine whether the player has the required PassLine and Come bets.

@amortization Wouldn't subname still be needed for the come bets? I agree that subname is clunky, and it really only exists to distinguish the bets where a user can have a similar bet on multiple numbers (i.e. Come, DontCome, Odds).

My only question is how this will be accessed in strategy.py. For example, when adding odds to the last come bet that moves to a number, what would that logic look like? How would we change passline_odds()? My preference is for the strategies to be easy to code to encourage users to build their own; as long as I can see a way forward with that, I'm all for it

[amortization]

I branched from this one since removing the subname had a lot more changes to player and strategy that aren't necessary for these changes to mainly just Bet. Those changes are in #20

[amortization]

I merged in #20

[amortization]

Before merging (I had forgotten about your comment regarding merging before changing this) I changed the Player module to be a part of Table and to only be created via table.add_player with table.add_player changed to take the player arguments.

[amortization]

@skent259 can you please check all of the added tests and make sure they're working as one would expect? Specifically the test_strategies_compare_bets? When creating #27 I reread some of the strategies and changed some of the tests to fit. For instance, I'm pretty sure that

(ironcross, {'mult': '2'}, [(4, 4)], {('PassLine', '', 5),
                                      ('Odds8', '', 10),
                                      ('Place5', '', 5),
                                      ('Place6', '', 6),
                                      ('Field', '', 5)}),

should actually be:

(ironcross, {'mult': '2'}, [(4, 4)], {('PassLine', '', 5),
                                         ('Odds8', '', 10),
                                         ('Place5', '', 10),
                                         ('Place6', '', 12),
                                         ('Field', '', 5)}),

and I may have misread that strategy when designing the tests or making changes to Strategy. I just want to make sure that those are all correct since that's what we're making changes on.

[skent259]

@skent259 can you please check all of the added tests and make sure they're working as one would expect? Specifically the test_strategies_compare_bets? When creating #27 I reread some of the strategies and changed some of the tests to fit. For instance, I'm pretty sure that

(ironcross, {'mult': '2'}, [(4, 4)], {('PassLine', '', 5),
                                      ('Odds8', '', 10),
                                      ('Place5', '', 5),
                                      ('Place6', '', 6),
                                      ('Field', '', 5)}),

should actually be:

(ironcross, {'mult': '2'}, [(4, 4)], {('PassLine', '', 5),
                                         ('Odds8', '', 10),
                                         ('Place5', '', 10),
                                         ('Place6', '', 12),
                                         ('Field', '', 5)}),

and I may have misread that strategy when designing the tests or making changes to Strategy. I just want to make sure that those are all correct since that's what we're making changes on.

@amortization Definitely, I will take a look. Been a bit busy but I will make time for this today or tomorrow.

[skent259]

@amortization From the looks of it, both the ironcross() and hammerlock() strategies are slightly off now. I fixed the tests in 50101e6, but didn't fix the strategies yet. For the current code to work, I think there needs to be a unit argument to the core strategies like place.

Also, can you update amortization:bet_changes to skent259:bet_changes, so the files changed load correctly on this? Thank you! I'll do a full review after that, but I think this is getting close