skent259 / crapssim

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

Add a Strategy object type #23

Closed amortization closed 8 months ago

amortization commented 2 years ago

I think that it would make sense for us to have a Strategy base class that could be derived from to create strategies.

Currently, for strategies that have attributes that need to be stored those attributes are passed to the player and stored as strat_info. These items then have to be passed back from the player to the strategy with a kwargs argument as it isn't always apparent what attributes will need to be passed.

If each players strategy was it's own object, the attributes could all be defined and stored by the Strategy which I think will be easier for end users creating strategies to understand. It would also let us remove unit from player, and just allow each strategy to define it's own init method with the necessary arguments allowing more flexibility.

It would also let us break up complex strategies into multiple smaller methods within the Strategy for code readability and sustainability.

How I envision the base class looking is

class Strategy(ABC):
    @abstractmethod
    def update_bets(self, player: Player, table: Table):
        pass

Then have each strategy subclass from it. For strategies without complex inputs or outputs, it will be as simple as taking what is already defined in the method and adding necessary inputs. For example with passline:

class PassLineStrategy(Strategy):
    def __init__(self, bet_amount: typing.SupportsFloat):
        self.bet_amount = bet_amount

    def update_bets(self, player: Player, table: Table):
        if table.point == "Off" and not player.has_bets(PassLine):
            player.place_bet(PassLine(self.bet_amount), table)

As you can see, it separates the unit parameter from the player and allows any bet amount to be used for the Strategy. This way if the player wanted to use a strategy where they place $5 dollar pass line bets but $100 come bets they wouldn't be beholden to the player.unit when re-using strategies.

I think that it would really allow you to simplify strategies that are required to store information. For example hammerlock could look something like (untested and just for illustration):

class HammerLock(Strategy):
    def __init__(self, bet_amount, win_mult="345"):
        self.bet_amount = bet_amount
        self.win_mult = win_mult
        self.mode = None

    def update_bets(self, player: Player, table: Table):
        PassLineStrategy(self.bet_amount).update_bets(player, table)
        LayOddsStrategy(self.bet_amount).update_bets(player, table)

        self.update_mode(player, table)

        if self.mode == 'place68':
            PlaceStrategy(self.bet_amount, numbers={6, 8}, skip_point=False).update_bets(player, table)
        elif self.mode == 'place_inside':
            PlaceStrategy(self.bet_amount, numbers={5, 6, 8, 9}, skip_point=False).update_bets(player, table)
        elif self.mode == 'takedown':
            self.takedown(player)

    def update_mode(self, player: Player, table: Table):
        if self.mode is None and table.point.status == 'On':
            self.mode = 'place68'
        elif self.mode == 'place68' and not player.has_bets(Place6, Place8):
            self.mode = 'place_inside'
        elif self.mode == 'place_inside' and not player.has_bets(Place5, Place6, Place8, Place9):
            self.mode = 'takedown'

    def takedown(self, player):
        for bet_type in (Place5, Place6, Place8, Place9):
            player.remove_if_present(bet_type)

We could then add other object related methods to the Strategy if we wanted, for instance we could have a add method that could combine multiple strategies so something like the PassLine(5) and Come(100) described above could be represented as just PassLine(5) + Come(100).

Thoughts on this implementation of strategy?

amortization commented 2 years ago

Another potential benefit came up in #20 in that you could have different hooks for the strategy at different stages of the betting process. For example it could be:

class Strategy(ABC):
    def before_roll(self, player, table):
        """Method that can update the Strategy from the table/player before the dice are rolled."""
        pass

    def after_roll(self, player, table):
        """Method that can update the Strategy from the table/player after the dice are rolled but
        before the bets are updated."""
        pass

    def after_bets_updated(self, player, table):
        """Method that can update the Strategy from the table/player after the bets on the table
         are updated but before the table is updated"""
        pass

    def after_table_update(self, player, table):
        """Method that can update the Strategy from the table/player after the table is updated."""
        pass

    @abstractmethod
    def update_bets(self, player, table):
        """Add, remove, or change the bets on the table."""
        pass

The Strategy wouldn't have to override those methods, however, they would be able to if desired to update itself with attributes from the table/player/bets at different stages of the roll. Then in the Table you would just call these methods in the appropriate spots in order for the correct attributes to be set.

skent259 commented 2 years ago

I have to think more about this. To people like you and me, a strategy class makes perfect sense and has several benefits that you have outlined. However, I worry that it represents a massive increase in programming knowledge required to implement a strategy.

Consider an updated hammerlock that follows some of your logic but without the strategy class:

def hammerlock(player: 'Player', table: 'Table', mode: str | None = None) -> dict[str, str]:
    passline(player, table)
    layodds(player, table, win_mult="345")

    if mode is None and table.point.status == 'On':
        mode = 'place68'
    elif mode == 'place68' and not player.has_bets(Place6, Place8):
        mode = 'place_inside'
    elif mode == 'place_inside' and not player.has_bets(Place5, Place6, Place8, Place9):
        mode = 'takedown'

    if mode == 'place68':
        place(numbers={6, 8}, skip_point=False)
    elif mode == 'place_inside':
        place(numbers={5, 6, 8, 9}, skip_point=False)
    elif mode == 'takedown':
        for bet_type in (Place5, Place6, Place8, Place9):
            player.remove_if_present(bet_type)

    player.strat_info['mode'] = mode

This is functionally the same thing but doesn't require a user to understand when to use self, and how to add functions (though you certainly could add update_mode() within this function to clean things up somewhat). They don't need an understanding of classes or inheritance and don't need to know which methods to override. Since python is primarily a scripting language, I don't want to assume that people have knowledge of all these advanced programming topics.

That being said, I see the value in potentially adding a bet_amount variable to strategies, and see how this could simplify some strategies in the future. So, long story short, I need to think more about the pros and cons of each approach.

amortization commented 2 years ago

I don't think that it would really cause too much confusion for end users as long as it's documented. Any user who is able to install the crapssim module should be able to subclass an object. I'm far from an expert or professional programmer but think that it wouldn't be any more difficult to comprehend than passing a dictionary through a method. Plus they're already having to understand some level of inheritance and OO since they are having to interact with the player, table, point and bet objects to set up any strategy that has even moderate complexity.

I think in a lot of cases it would greatly simplify things for end users, as knowing when in the process the info is updated, and giving access to info at different points in the process would allow more complex strategies to be understood at a simpler level with method documentation for each subclass that utilizes them. Currently it would take a lot of inline documentation to explain a strategy step by step.

For users who just want simple strategies based on table or whatever they could write it just as before, only overriding the update_bet method instead of creating a new method.

For the most basic of end users if the Strategy had an add method as well could set up basic Strategy implementations that are easily accessible, reducing the need for any programming down to almost nothing, and then give them access to those. For example:

class PointOnBet(Strategy):
    """Add Bet when Point is On"""
    def __init__(self, bet_type, bet_amount):
        self.bet_type = bet_type
        self.bet_amount = bet_amount

    def update_bet(self, player, table):
        if table.point.status == 'On':
            player.place_bet(self.bet_type(self.bet_amount), table)

class PointOffBet(Strategy):
    """Add Bet when Point is Off"""
    def __init__(self, bet_type, bet_amount):
        self.bet_type = bet_type
        self.bet_amount = bet_amount

    def update_bet(self, player, table):
        if table.point.status == 'On':
            player.place_bet(self.bet_type(self.bet_amount), table)

Then all the end user would need to do if add is overridden by Strategy is:

player.strategy = PointOffBet(PassLine, 5) + PointOnBet(Field, 15) + PointOnBet(Hard4, 10)

Which I think is easier than defining a method that does the same thing. Any number of these could be easily created for any number of simple scenarios making it easy for non programmers to utilize as well.

skent259 commented 2 years ago

These are all fair points. It seems that with this approach (and a fair bit of coding), we could create something where the strategies are:

Thus, any change like this would have to come with a good set of documentation and potentially some walkthroughs.

I'd still like to think about it more, but my current position would be to implement something like this for v0.4.0 or later. I'd like to have a released version that has the important changes to bet, table, player, etc that are done/in progress with the current strategy syntax. Separating this change from the others will also highlight that it is a fundamental change in the package structure, which I think is important

skent259 commented 8 months ago

Added with #29, but possibly some additional work to be done. Closing