Snowflake singleplayer implementation #57
Conversation
Bumps [redis](https://github.com/redis/redis-py) from 5.0.7 to 5.0.8. - [Release notes](https://github.com/redis/redis-py/releases) - [Changelog](https://github.com/redis/redis-py/blob/master/CHANGES) - [Commits](redis/redis-py@v5.0.7...v5.0.8) --- updated-dependencies: - dependency-name: redis dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bump redis from 5.0.7 to 5.0.8
Bumps [sqlalchemy](https://github.com/sqlalchemy/sqlalchemy) from 2.0.31 to 2.0.32. - [Release notes](https://github.com/sqlalchemy/sqlalchemy/releases) - [Changelog](https://github.com/sqlalchemy/sqlalchemy/blob/main/CHANGES.rst) - [Commits](https://github.com/sqlalchemy/sqlalchemy/commits) --- updated-dependencies: - dependency-name: sqlalchemy dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…2.0.32 Bump sqlalchemy from 2.0.31 to 2.0.32
Bumps [twisted](https://github.com/twisted/twisted) from 24.3.0 to 24.7.0. - [Release notes](https://github.com/twisted/twisted/releases) - [Changelog](https://github.com/twisted/twisted/blob/trunk/NEWS.rst) - [Commits](twisted/twisted@twisted-24.3.0...twisted-24.7.0) --- updated-dependencies: - dependency-name: twisted dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bump twisted from 24.3.0 to 24.7.0
Fix `tile_range` logic for tusk
Bumps [sqlalchemy](https://github.com/sqlalchemy/sqlalchemy) from 2.0.32 to 2.0.33. - [Release notes](https://github.com/sqlalchemy/sqlalchemy/releases) - [Changelog](https://github.com/sqlalchemy/sqlalchemy/blob/main/CHANGES.rst) - [Commits](https://github.com/sqlalchemy/sqlalchemy/commits) --- updated-dependencies: - dependency-name: sqlalchemy dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…2.0.33 Bump sqlalchemy from 2.0.32 to 2.0.33
Bumps [sqlalchemy](https://github.com/sqlalchemy/sqlalchemy) from 2.0.33 to 2.0.35. - [Release notes](https://github.com/sqlalchemy/sqlalchemy/releases) - [Changelog](https://github.com/sqlalchemy/sqlalchemy/blob/main/CHANGES.rst) - [Commits](https://github.com/sqlalchemy/sqlalchemy/commits) --- updated-dependencies: - dependency-name: sqlalchemy dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…2.0.35 Bump sqlalchemy from 2.0.33 to 2.0.35
Bumps [redis](https://github.com/redis/redis-py) from 5.0.3 to 5.0.4. - [Release notes](https://github.com/redis/redis-py/releases) - [Changelog](https://github.com/redis/redis-py/blob/master/CHANGES) - [Commits](redis/redis-py@v5.0.3...v5.0.4) --- updated-dependencies: - dependency-name: redis dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
|
Time to take a look at this... |
There was a problem hiding this comment.
This looks pretty good so far. I did some cleanup, and the only issue I encountered was, that after some time, the ninjas would just not attack anymore. However, this could have been an issue on my end, as I have no idea how to reproduce this issue yet.
Please take a look at the reviews, when you get time!
| self.power_card_slots: List[CardObject] = [] | ||
| self.power_cards: List[CardObject] = [] | ||
| self.owned_cards: List[CardObject] = [] | ||
| self.cards_ready: int = 0 |
There was a problem hiding this comment.
Please explain the differences between the previous card system and the new one. What does it help with?
| if self.cards_ready >= 4: | ||
| self.logger.info(f'{self.name} needs to place a card before queuing a new one!') | ||
| return |
There was a problem hiding this comment.
Why is this there? This should get handled below, with the cycle update attribute:
if len(self.owned_cards) > 3:
update['cycle'] = TrueThere was a problem hiding this comment.
Could this check be added to the next_power_card function, like this?
def next_power_card(self) -> CardObject | None:
if not self.has_power_cards:
# Client has no more power cards
self.logger.info(f'{self.name} has no more power cards')
return
if self.cards_ready >= 4:
self.logger.info(f'{self.name} needs to place a card before queuing a new one!')
return
next_card = random.choice(self.owned_cards)
self.owned_cards.append(next_card)
self.owned_cards.remove(next_card)
return next_cardbecause this would mean that the stamina bar still goes up, like it used to.
There was a problem hiding this comment.
Yes. But why should the stamina bar go up if the card queue is full? Wouldn't this mean you 'waste' stamina which could be used on a card because your queue is full?
I also don't see why we're appending to the end of self.owned_cards and removing the same entry?
Wouldn't this leave the list as it is?
There was a problem hiding this comment.
The .append & .remove was there in your original code, so I am not sure if this is intended.
The stamina bar is supposed to go up, even if you have all slots filled up already. But don't worry, this doesn't mean they "waste" anything. This is where the cycle attribute comes in. This one just resets the stamina back to 0, without giving you any card, so that it can go back up again. If you want I could try to find a video for this, but its very rare to see someone having 4 powercards at once.
| def is_ninja_selected(self, ninja: Ninja) -> bool: | ||
| return any(ninja.selected_object == ninja for ninja in self.game.ninjas) |
There was a problem hiding this comment.
Please check if this section works correctly. The ninja parameter might get overridden inside the for-loop.
|
Okay, I have debugged this a bit and it seems like the snow ninja's selection doesn't work anymore after a while. If you have time, maybe investigate this a bit. |
|
@Walinski Please address the requested changes if you get time. Thanks! |
|
Sorry that the review took so long, we are almost done! |
Lekuruu
changed the title
No description provided.