kipyin / pokemaster Goto Github PK

Feature Description and Rationale

Is your feature request related to a problem? Please describe.

Currently, using pokemaster.prng.PRNG to generate a random number requires manual calculations. This is error-prone. For example, in the following code, what I've written generates a random number between [0, 1] inclusive on both ends:

Whereas random functions normally generate numbers between [0, 1), a left-close-right-open range.

Describe the solution you'd like

Adding a random() method to pokemaster.prng.PRNG class will make getting random numbers with that PRNG much easier.

We can also add:

• PRNG().choice(items) randomly selects an item from items;
• PRNG().shuffle(items) returns the elements in items but in random order;
• PRNG().randint(a, b) returns a random integer between a and b.

and more functions from the random standard library, if needed.

Usage Example

>>> from pokemaster.prng import PRNG
>>> prng = PRNG()
>>> prng.random()
0.0
>>> prng.random()
0.912078857421875
>>> prng.uniform(0.85, 1.0)
0.87911376953125

Suggested Implementation/Change

In pokemater.prng, add the following methods to PRNG class:

class PRNG:
    ...
    def random(self) -> float:
        """Return a random number from the uniform distribution [0, 1)."""
        return self.next() / 0x10000

    def uniform(
        self, min: Union[int, float] = None, max: Union[int, float] = None
    ) -> float:
        """Return a random number from the uniform distribution [min, max)

        Usage::

            PRNG.uniform() -> a random number between [0, 1)
            PRNG.uniform(n) -> a random number between [0, n)
            PRNG.uniform(m, n) -> a random number between [m, n)

        """
        if min is not None and not isinstance(min, Real):
            raise TypeError(f"'min' must be an int or a float.")
        if max is not None and not isinstance(max, Real):
            raise TypeError(f"'max' must be an int or a float.")

        if min is None and max is None:
            # PRNG.uniform() -> [0, 1)
            return self.random()
        elif (min is not None and max is None) or (
            min is None and max is not None
        ):
            # PRNG.uniform(n) -> [0, n)
            cap = min or max
            return self.random() * cap
        else:
            # PRNG.uniform(m, n) -> [m, n)
            if max <= min:
                raise ValueError("'max' must be strictly greater than 'min'.")
            return self.random() * (max - min) + min

Binding a pokemaster.pokemon.Pokemon object to a db session couples the class with a db schema, and having them completely separated is much preferred.

But if we remove this class var, how can properly instantiate a pokemaster.pokemon.Pokemon instance?

E.g., how do we get the correct database for the following helper class method?

Issue #15 suggested the implementation of in-battle stats, but none of the then-existing codes actually used it, hence there is a few things that I was missing when I wrote BattleStats:

1. The stage multipliers needs to be tracked, because some moves (e.g. Punishment) need to use this info.

2. The stage multipliers actually also include the critical-hit rate, which was not included when implementing BattleStats.

Hence, to implement above mechanisms, we have two choices:

1. Make a new class, StatStages, and put the stage info into this new class.

2. Make a _Stat class, which represent a single stat, such as hp or attack, which contains the value of its permanent stats, effective stats, stage, and stage change history. Remove BattleStats class. Access all stat-related data like Pokemon().stats.hp.permanent, Pokemon().stats.evasion.effective, Pokemon().stats.critical_hit.stage_log, etc. Of course, Pokemon().stats.critical_hit.permanent will be a non-sense, and will probably be 0.

Here is a scratch of how implementing items using classes should look like:

@attr.s
class Item:
    """An Item in the Pokémon world."""

    cost: int = attr.ib(init=False)
    sell_price: int = attr.ib(init=False)
    pocket: str = attr.ib(init=False)


@attr.s(slots=True)
class _Medicine(Item):

    cost: int = attr.ib(init=False)
    sell_price: int = attr.ib(init=False)
    pocket: str = attr.ib(init=False)
    recover: int = attr.ib(init=False)

    def battle_effect(self, target):
        target.current_hp = min(target.permanent_stats.hp, self.recover)

    def field_effect(self, target):
        self.battle_effect(target)


@attr.s
class Potion(_Medicine):

    def __attrs_post_init__(self):
        self.cost = 300
        self.sell_price = self.cost // 2
        self.pocket = 'items'
        self.recover = 20


@attr.s
class SuperPotion(_Medicine):

    def __attrs_post_init__(self):
        self.cost = 700
        self.sell_price = self.cost // 2
        self.pocket = 'items'
        self.recover = 50

Now, we should be able to do the following:

>>> from pokemaster.pokemon import Pokemon
>>> from pokemaster.item import SuperPotion
>>> super_potion = SuperPotion()
>>> p = Pokemon(national_id=1, level=100)
>>> print(f'The original HP: {p.current_hp}')
The original HP: 216
>>> p.current_hp = 0
>>> print(f'The HP should be 0: {p.current_hp}')
The HP should be 0: 0
>>> super_potion.battle_effect(p)
>>> print(f'HP after recovered by {super_potion}: {p.current_hp}')
HP after recovered by SuperPotion(cost=700, sell_price=350, pocket='items', recover=50): 50

Is this a good enough solution to carry on?

I've seen the attrs team using towncrier to generate their CHANGELOG, and after looking into towncrier's docs, I think it is the go-to tool for CHANGELOG management.

Using towncrier

towncrier uses news fragments to store changes between each release. A piece of news fragment is a rST-style file with suffixes like .feature, .bugfix, etc.

When publishing a new release, all news fragments will be moved to the corresponding sections in CHANGELOG.md, and the fragments will be removed (by using towncrier command).

All news fragments will be stored in the newsfragments directory under the project root.

Detailed Steps

• Add towncrier to the dev-dependency (poetry add towncrier -D).
• Add a newsfragments to the project root.
• Add .gitignore to newsfragments/.gitignore.
• Add a [tool.towncrier] section to pyproject.toml.
• Run towncrier command to generate the CHANGELOG for version 0.1.3.

I use a centralized pokemaster.database to manage all the query functions. But I think all functions in pokemaster.database should not be used by the end users directly. Hence, in the most recent commit 586d8ca, I've made it a private module, but I'm not entirely sure about this change.

Making pokemaster.database a private module makes perfect sense. The only thing the end users should use is really just pokemaster.Pokemon. The end users should not worry about all the SQLAlchemy tables under the hood. Only the vanilla Python types should be used as the arguments for the public APIs.

If there is no downside of having the module to be private, I'll include this change in release v0.1.

pip install pokemaster does not work:

$ pip install pokemaster
...
Collecting pokedex (from pokemaster)
  Could not find a version that satisfies the requirement pokedex (from pokemaster) (from versions: )
No matching distribution found for pokedex (from pokemaster)
...

I've searched all existing issues on poetry and cannot find a solution to this (or even a relevant issue). I think pokedex should be shipped with pokemaster.

Issue Description

When a Pokémon tries to learn a new move (either through level-up or use a machine), if the Pokémon knows an HM move, it will be eventually forgotten.

Minimum Crash Example

>>> from pokemaster import Pokemon
>>> mew = Pokemon('mew', level=10)
>>> for hm in range(101, 105):
...    mew.use_machine(hm)
>>> mew.moves
deque(['cut', 'fly', 'surf', 'strength'], maxlen=4)
>>> mew.use_machine(105)
deque(['fly', 'surf', 'strength', 'flash'], maxlen=4)

Change growth_rate_id to species_identifier will make this function much easier (and clearer) to use.

Currently, the Pokemon class will enforce all attributes and methods conform with the official data, and the users are not able to alter the attribute values whatsoever.

Conforming to the official data is a good practice, but what if the "official data" is wrong? Or what if the users want to make their own Pokémon world? In these cases, with the current design, the users only have one option: make and use their own database. This option will solve the first problem, but will not always solve the second, because the users have to use the database schema that veekun has designed.

Adding a Pokemon.OFFICIAL class variable as a switch will solve this issue:

class Pokemon:
    OFFICIAL = True
    def __init__(...):
        ...

In the methods, we can also add an if-else block that checks the setting:

class Pokemon:
    ...
    @property
    def ability(...):
        if self.OFFICIAL:
            return get_ability_from_db(...)
        else:
            return self._ability

Currently, the stats (Pokemon.stats) are so-called "Permanent Stats" (see Bulbapedia entry), and they should only change when a Pokémon levels up, or taking a Vitamin. We need to add another attribute to Pokemon to manage the in-battle statistics, which are all 6 permanent stats + Evasion and Accuracy.

Implementing the in-battle stats should be easy, just add a BattleStats class in pokemaster.stats module:

@attr.s(auto_attribs=True, slots=True)
class BattleStats:
    hp: float
    attack: float
    ...
    accuracy: float
    evasion: float

More methods can be added as we implement the battle engine.

Feature Description and Rationale

The game Ruby (version ID 7) is a Generation 3 game, and it belongs to the "version group" ruby-sapphire (version group ID 5). Currently, the only way to get the "version group" name and ID mapping is via tables such as pokedex.db.tables.VersionGroup or pokedex.db.tables.Version. But making a query every time we want to retrieve the data is unnecessarily costly. Thus, adding a GameVersion class will help getting the right version/version group ID at ease.

Usage Example

>>> from pokemaster.game_version import GameVersion
>>> GameVersion.EMERALD.generation
<Gen.THREE: 3>
>>> # What's the version group ID for Platinum?
>>> GameVersion.PLATINUM.version_group
<VersionGroup.PLATINUM: 9>

Implementation Suggestions

First, we need a Gen enum and VersionGroup enum:

class Gen(enum.IntEnum):
    ONE = 1
    TWO = 2
    THREE = 3
    ...

class VersionGroup(enum.IntEnum):
    RED_BLUE = 1
    YELLOW = 2
    GOLD_SILVER = 3
    ...

Then, we can make a GameVersion enum that combines Gen and VersionGroup:

class GameVersion(enum.Enum):
    RED = (1, 1, 1)
    BLUE = (1, 1, 2)
    YELLOW = (1, 2, 3)
    ...
    def __init__(self, generation, version_group, version):
        self.generation = Gen(generation)
        self.version_group = VersionGroup(version_group)
        self.version = version

And then we can do something like:

>>> Gen(3) == Gen.THREE
True
>>> VersionGroup(17)
<VersionGroup.SUN_MOON: 17>
>>> GameVersion.PLATINUM
<GameVersion.PLATINUM: (4, 9, 14)>

If we make the values of GameVersion to namedtuple, we can even have something more descriptive:

>>> GameVersion.PLATINUM
<GameVersion.PLATINUM: VersionTuple(generation=4, version_group=9, version=14)>

File pokemon.py has 331 lines of code (exceeds 250 allowed). Consider refactoring.

https://codeclimate.com/github/kipyin/pokemaster/pokemaster/pokemon.py#issue_5c3b670f262108000100001b

I've been debating about this some time ago, and this choice has come to me recently once again (the use-invoke branch is still active lol).

Anyway, the outcomes are the same regardless of the tool.

Pros for using invoke:

• It behaves the same regardless of the platform!
• It's in Python.
• So people know how to use it as long as they know how to use Python.
• Better readability with docstrings.
• Complex commands can be easily written.
• No tabs.

Pros for using Makefile:

• I'm already using it.
• Since invoke and Makefile are pretty much the same, why bother.
• Adding a make.bat for Windows is about the same workload as switching to invoke.
• Makefile is much more widely used and popular than invoke (so what?).
• Not that hard to learn for people who don't know.

I prefer using invoke.

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."""
        ...

I've been scratching my head on this for about a month, and I still can't find a good design for the most basic Pokemon class.

On one hand, I want Pokemon to be dead simple to use. One should expect to do this without any worry:

>>> bulbasaur = Pokemon('bulbasaur', level=5)

and everything about a Bulbasaur will be created, such as its IVs, stats, ability, nature, gender, etc. But the price of this convenience is binding the database schema to Pokemon.__init__().

This brings me to my next point, although I've said it before (in #1): I want to have Pokemon separated from veekun's pokedex project. pokedex is a great thing, and I can't imagine having a better one. But it has some critical issues, such as the Pokémon's base experience (i.e. the experience some Pokémon gains upon defeating a Pokémon) and items' costs are only valid for Gen. VII. Also, decoupling the class with the database gives the class more flexibility. What if someone wants to use pokemaster to make their own Pokémon game? Having the class coupled to a fixed database is definitely not a good design.

The attrs team suggests using @classmethod as a helper function to instantiate the class. But if we do so, the convenience will be sacrificed. Doing something like Pokemon.new() is truly cumbersome.

Also, I don't think it is possible to have a Pokemon class that is completely separated from a database. The evolution data has to come from somewhere. Pokémon's nature, ability, moves, are tightly related to the exact game version. Pokemon has different attributes for different games. All of these should be considered when we are trying to come up with some kind of "blank" or fit-all Pokemon design. From this point of view, I think having the class coupled with a database is inevitable. But I'm still open for other possibilities.

Function __init__ has 9 arguments (exceeds 4 allowed). Consider refactoring.

https://codeclimate.com/github/kipyin/pokemaster/pokemaster/pokemon.py#issue_5c570c605bdaea0001000017

Feature Description and Rationale

Weathers (on Bulbapedia) are part of the battle mechanisms. By changing the battle environment, they can "activate Abilities, modify certain moves, and potentially damage the Pokémon in battle or affecting their stats." In the games, weathers have a duration, which determines how long the weather lasts.

Usage Examples

>>> from pokemaster.weather import Weather
>>> # Creates a 'Harsh Sunlight' weather that lasts 5 turns:
>>> harsh_sunlight = Weather('Harsh Sunlight', duration=5)
>>> harsh_sunlight
Weather(name='harsh-sunlight', duration=5)
>>> # Default weather
>>> clear_skies = Weather('clear-skies')
>>> clear_skies
Weather(name='clear-skies', duration=inf)

Implementation Suggestions

@attr.s(auto_attribs=True, slots=True)
class Weather:
    name: str = attr.ib(
        validator=weather_name_validator, converter=weather_name_converter
    )
    duration: Union[int, float] = float('inf')