kipyin / pokemaster

Checkout kipyin/pokemaster2 !
https://github.com/kipyin/pokemaster2
2 stars 2 forks source link

Combine `PermanentStats`, `SpeciesStrengths`, and `NatureModifiers` into a single `Stats` class #5

Closed kipyin closed 5 years ago

kipyin commented 5 years ago

The Idea

Since PermanentStats, SpeciesStrengths, and NatureModifiers do not have special checks (unlike EV and IV), they can be combined into a single Stats class.

Potential Problems

If we do so, I believe we have to change how updating the permanent stats works. Currently, I'm using methods PermanentStats.level_up() and PermanentStats.evolve() to update the stats. If we combine all the stats into one class, these methods no longer make sense. Although I don't really like the design of the update mechanisms. The workaround is when a Pokémon levels up or evolves, just calculate and assign a new Stats class to its Pokemon.permanent_stats attribute.

PermanentStats._metadata is also a pain and a bad design. It exists solely to make my life a bit easier when implementing Pokemon.level_up() and Pokemon.evolve().

I should really just use dictionaries to represent all of the stats. Or named tuples (aka vectors, which is what I think the stats really are.)

Implementation Details

Probably something like this:

@attr.s(auto_attribs=True)
class Statistics:
    hp: Union[int, float]
    attack: Union[int, float]
    defense: Union[int, float]
    special_attack: Union[int, float]
    special_defense: Union[int, float]
    speed: Union[int, float]

    @classmethod
    def make_nature_modifiers(cls, nature: tb.Nature) -> 'Statistics':
        """Create a NatureModifiers instance from the Nature table."""
        ...

    @classmethod
    def make_species_strengths(cls, stats: List[tb.PokemonStat]) -> 'Statistics':
        """Create an instance of SpeciesStrengths from PokemonStat table."""
        ...
kipyin commented 5 years ago

https://github.com/kipyin/pokemaster/blob/f101f398ebda33e0e3a8a30c2548124fb08a7418/pokemaster/stats.py#L328-L331

Should use a proper for-loop.

kipyin commented 5 years ago

For EV and IV, maybe we can add an additional parameter, validation, whose value can be a StatisticsValidation enum to our Statistics class. A StatisticsValidation enum has two possible values: EV and IV, i.e.:

class StatisticsValidation(enum.Enum):
    EV = "EV"
    IV = "IV"

When Statistics.validation is set to IV, the class itself will ensure each stat does not exceed 32 upon assignment, by calling the correct validation function.

kipyin commented 5 years ago

I'm liking this idea a lot. I'm almost done implementing Stats, and just have some messes need to be cleaned up:

  1. Validation for EV's. Should we make a Stats().set_ev() to safely set the EV's?
  2. Stats is immutable right now. Species strengths (aka base stats), IV's, and nature modifiers are immutable, but permanent stats and EV's can be mutable (shouldn't permanent stats also be immutable, since it is 'permanent'?). A Pokémon's EV's change every time it wins a battle or takes a Vitamin in Gen. III. If we allow something like Stats().set_ev(), then it implies that Stats should be mutable. However, it doesn't make sense to have mutable species strengths, IV's, and nature modifiers, since they don't change. Which one should it be? Mutable or immutable? Also note that the immutability can be circumvented by attr.evolve().
  3. If Stats is immutable, then setattr() won't work in Stats.make_nature_modifiers(). (Fixable. Just use return cls(**kwargs) where kwargs keeps track of the nature modifiers.)
kipyin commented 5 years ago

bad4228a154e82317cbd385d66be71724d80e090 finishes the new Stats implementation. Closing this issue.