search

skent259 / crapssim

Simulator for craps with various betting strategies
MIT License
30 stars 14 forks source link

Table payouts should be improved for consistency #13

Closed skent259 closed 8 months ago

skent259 commented 2 years ago

There are many craps bets that have variable payouts at different casinos. How can we best incorporate this feature into crapssim?

List of bets that this involves:

Other related bet issues

skent259 commented 2 years ago

The current implementation creates awkward code sections in strategy.py, such as

https://github.com/skent259/crapssim/blob/88d36ca2a844a697d6751b281715b262ec528a38/crapssim/strategy.py#L386-L392

One thought is to remove the initialization option from the bet class and instead pull in that information only from table. This would require some pointer to the table on initialization. Then just put everything into table.payouts

amortization commented 2 years ago

Originally I wanted each bet to have a payout ratio based on the dice total, however given that these change (ex. PassLine 7 whether point is on or off) or they aren't always based on the dice total (ex. hard ways) I don't think that's completely feasible without lookup tables with a lot of variables available.

What if instead we removed bet.payoutratio from Bet entirely and replaced it with an overridable method on bet with table as a parameter, or ideally if we made Table an attribute of Bet use that.

That way each Bet would be able to look at the table for any overriding fields in the payout table, but still leaving the logic and the ratio on the bet itself. For example:

Table (making the attribute private with separate methods to change to control available values):

self._payouts: dict[str, typing.Any] = {'field_ratios': {2: 2, 3: 1, 4: 1, 9: 1, 10: 1, 11: 1, 12: 2},
                                                           'fire_points': {4: 25, 5: 100, 6: 1000}}

Field:

def get_payout_ratio(self):
    return self.table.payouts['field_payouts'][self.table.dice.total]

Fire:

def get_payout_ratio(self):
    ...
    return self.table.payouts['fire_points'][len(self.points_made)]

I think that this has the advantage that all of the payout criteria will be standardized, stored, and accessible via the Table, but will be easy to add new bets and criteria by just overriding the get_payout_ratio method on the bet.

skent259 commented 2 years ago

I really like the simplicity of your suggestion for the field and fire bets. However, I'm wondering if there's a way to keep the clean syntax that the bet.payoutratio option provides, such as in:

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

Could we accomplish something similar by just changing the _update_bet() method in the specific bets where it matters? For the field, and using your logic, this might look like:

def _update_bet(self, table_object: "Table", dice_object: Dice) -> tuple[str | None, float]:
    status: str | None = None
    win_amount: float = 0

    if dice_object.total in self.winning_numbers:
         payoutratio =  self.table.payouts['field_payouts'][self.table.dice.total] 
         win_amount = payoutratio * self.bet_amount
    elif dice_object.total in self.losing_numbers:
         status = "lose"

    return status, win_amount

Actually, this could be done without requiring self.table.payouts and could use table_object.payouts instead now that I think about it.

amortization commented 2 years ago

I think that the issue I see with that is that we are then going deeper down the rabbit hole of having parent attributes that are unused by the child. In this case the Bets payoutratio would still be included on the children, even though it is not used/needed on all children. I think that it would be analogous to having a Animal type with a wingspan attribute even though Horse(Animal) wouldn't want to have a wingspan attribute.

In your example, you could still have payoutratio on the Place class, and be able to leave Place4, 5 etc. alone:

class Place(Bet):
    def __init__(self, bet_amount: float):
        super().__init__(bet_amount)
        self.payoutratio: float = 1.0

    def get_payout_ratio(self):
        return self.payoutratio

The reason I went with the self.table approach instead of passing it as a parameter was due to our discussion in #3 about making the Bet references to Table consistent with the Player references to table as a pointer.

Regarding that if we went that route with a table pointer probably a better option might be to make payoutratio a property of Bet. This way you should be able to leave the existing bets, as they are, and just override the property on bets where you need a lookback to the table (or any other sort of advanced logic.) I think that this could be the best of both worlds. I'm going to explore this option and see how that might look.

While we're at it, I do think that we should refactor payoutratio to payout_ratio to be more PEP 8 compliant (https://peps.python.org/pep-0008/#function-and-variable-names)

skent259 commented 2 years ago

I definitely see your point about having consistent attributes for parent and child.

The reason I went with the self.table approach instead of passing it as a parameter was due to our discussion in https://github.com/skent259/crapssim/pull/3 about making the Bet references to Table consistent with the Player references to table as a pointer.

Regarding that if we went that route with a table pointer probably a better option might be to make payoutratio a property of Bet. This way you should be able to leave the existing bets, as they are, and just override the property on bets where you need a lookback to the table (or any other sort of advanced logic.) I think that this could be the best of both worlds. I'm going to explore this option and see how that might look.

While we're at it, I do think that we should refactor payoutratio to payout_ratio to be more PEP 8 compliant (https://peps.python.org/pep-0008/#function-and-variable-names)

I agree with all 3, and think the property route could be the way to go here. I think it reflects the reality that all bets have a payout_ratio, but the logic is more complex for some more than others.

amortization commented 2 years ago

Added Pull Request #18 with those changes.

I think that there are probably other attributes of Bet that we want to give similar a similar treatment.

One would be removable since the only bet that it is used on is the Passline, we could have it be:

@property
def removable(self):
    if self.table.point.status == "On":
        return False
    return True

Another could be name since in every case name matches the name of the class. To avoid having to re-enter the name of the bet on each class, on the Bet class we could just have:

@property
def name(self):
    return self.__class__.__name__

Another thing that may require some thought is I'm not sure how we want to handle winning and losing numbers. This really comes into play for the Hard Ways since for something like a Hard4 would 4 be considered a winning or a losing number? I think that it would be clearer if we pushed the winning and losing numbers down to only the subclasses that utilize them in their update logic.

amortization commented 2 years ago

I also don't know whether we would want to consider renaming Table.payouts to something like Table.settings. That way if we ever wanted to add more validation logic to the Table for things other than payouts we would be able to do so. Some examples of things that it could be utilized for would be:

amortization commented 2 years ago

I added #19 as a branch of #18 as well to implement some of the other changes above in regards to the class structure. I would like to tackle subname as well, however, I think that is going to be a little more difficult as we will probably want to refactor some of the ways bets are looked up by Player with get_bet method.

skent259 commented 8 months ago

Closed with #30