Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typehint warnings #86

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/game_entities/character.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def __init__(
strength,
"PHYSICAL",
strategy,
[1],
lvl,
skills,
alterations,
Expand All @@ -125,13 +126,13 @@ def __init__(
self.gold: int = gold
self.interaction: dict[str, any] = interaction
self.join_team: bool = False
self.reach_: Sequence[int] = [1]
self.constitution: int = (
Character.races_data[race]["constitution"]
+ Character.classes_data[classes[0]]["constitution"]
)
self._reach: Sequence[int] = [1]

def talk(self, actor: Entity) -> list[list[BoxElement]]:
def talk(self) -> list[list[BoxElement]]:
"""
Compute the dialog that should be displayed to the player when trying to interact with the entity.

Expand Down Expand Up @@ -248,7 +249,7 @@ def get_weapon(self) -> Optional[Weapon]:
Return the weapon born by the character.
"""
for equipment in self.equipments:
if equipment.body_part == "right_hand":
if equipment.body_part == "right_hand" and isinstance(equipment, Weapon):
return equipment
return None

Expand All @@ -257,12 +258,16 @@ def reach(self) -> Sequence[int]:
"""
Return the range of reach of the character.
"""
reach: Sequence[int] = self.reach_
reach: Sequence[int] = self._reach
weapon: Weapon = self.get_weapon()
if weapon is not None:
reach = weapon.reach
return reach

@reach.setter
def reach(self, value: Sequence[int]):
self._reach = value

@property
def attack_kind(self) -> DamageKind:
"""
Expand Down Expand Up @@ -370,7 +375,7 @@ def equip(self, equipment: Equipment) -> int:

def unequip(self, equipment: Equipment) -> bool:
"""
Handle the unequipment of the given equipment.
Handle the unequipping of the given equipment.
Return whether the equipment has been successfully unequipped or not.

Keyword argument:
Expand Down
24 changes: 0 additions & 24 deletions src/game_entities/consumable.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,3 @@ def __init__(
self.drink_sfx: pygame.mixer.Sound = pygame.mixer.Sound(
os.path.join("sound_fx", "potion.ogg")
)

def use(self, entity: Movable) -> tuple[bool, Sequence[str]]: # NOQA
"""
Apply the effects of the consumable to the entity that used it.

Return whether the consumable has been used (it can fail if conditions are not met) and the
messages that should be displayed on the interface.

Keyword arguments:
entity -- the entity that is trying to use the consumable
"""
success: bool = False
messages: list[str] = []
for effect in self.effects:
sub_success: bool
message: str
sub_success, message = effect.apply_on_ent(entity)
messages.append(message)
if sub_success:
success = True
if success:
pygame.mixer.Sound.play(self.drink_sfx)
entity.remove_item(self)
return success, messages
2 changes: 1 addition & 1 deletion src/game_entities/foe.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ def __init__(
strength,
attack_kind,
strategy,
reach,
lvl,
alterations=alterations,
)
self.reach: Sequence[int] = reach
self.xp_gain: int = int(xp_gain * (1.1 ** (lvl - 1)))
self.potential_loot: Sequence[tuple[Item, float]] = loot
self.keywords: Sequence[Keyword] = [] if keywords is None else keywords
Expand Down
23 changes: 19 additions & 4 deletions src/game_entities/movable.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class Movable(Destroyable):
target -- the target of the current attack if there is any
_attack_kind -- the kind of damage dealt by the entity
strategy -- the strategy of the entity if it's controlled by the AI
reach -- the range of reach of the entity
skills -- the list of skills of the entity
walk_sfx -- the sound started when the entity is moving
skeleton_sfx -- the sound started when a skeleton is moving
Expand Down Expand Up @@ -126,6 +127,7 @@ def __init__(
strength: int,
attack_kind: str,
strategy: str,
reach: Sequence[int],
lvl: int = 1,
skills: Optional[Sequence[Skill]] = None,
alterations: Optional[Sequence[Alteration]] = None,
Expand Down Expand Up @@ -159,6 +161,7 @@ def __init__(
DamageKind[attack_kind] if attack_kind is not None else None
)
self.strategy: EntityStrategy = EntityStrategy[strategy]
self.reach: Sequence[int] = reach
self.skills: Sequence[Skill] = skills

self.walk_sfx: pygame.mixer.Sound = pygame.mixer.Sound(
Expand Down Expand Up @@ -427,7 +430,7 @@ def set_item(self, item: Item) -> bool:
return False

# TODO: should return None if there is no Item found instead of -1
def remove_item(self, item_to_remove: Item) -> Union[Item, int]:
def remove_item(self, item_to_remove: Optional[Item]) -> Union[Item, int]:
"""
Remove the given item from the entity's inventory and return it.

Expand All @@ -442,13 +445,25 @@ def remove_item(self, item_to_remove: Item) -> Union[Item, int]:
def use_item(self, item: Consumable) -> tuple[bool, Sequence[str]]:
"""
Consume the given item.
Return whether the item has been used or not and
the sequence of messages that should be displayed to the player.
Return whether the consumable has been used (it can fail if conditions are not met) and the
messages that should be displayed on the interface.

Keyword arguments:
item -- the item that should be consumed
"""
return item.use(self)
success: bool = False
messages: list[str] = []
for effect in item.effects:
sub_success: bool
message: str
sub_success, message = effect.apply_on_ent(self)
messages.append(message)
if sub_success:
success = True
if success:
pygame.mixer.Sound.play(item.drink_sfx)
self.remove_item(item)
return success, messages

def move(self) -> None:
"""
Expand Down
5 changes: 2 additions & 3 deletions src/game_entities/shop.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from typing import Optional

import pygame.mixer
from lxml import etree
from pygamepopup.components import BoxElement, Button, InfoBox, TextElement

from src.game_entities.building import Building
Expand Down Expand Up @@ -124,7 +123,7 @@ def buy(self, item: Item) -> str:
Return the message that should be displayed to the player after the purchase tentative.

Keyword arguments:
actor -- the actor buying the item
actor -- the actor making the purchase
item -- the item that is being bought
"""
if self.current_visitor.gold >= item.price:
Expand Down Expand Up @@ -152,7 +151,7 @@ def sell(self, item: Item) -> tuple[bool, str]:
Return whether the item has been sold or not and the message that should be displayed to the player.

Keyword arguments:
actor -- the actor selling the item
actor -- the actor making the sale
item -- the item that is being sold
"""
if 0 < item.resell_price < self.shop_balance:
Expand Down
1 change: 0 additions & 1 deletion src/game_entities/skill.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from typing import Optional

from src.game_entities.alteration import Alteration
from src.services.language import TRANSLATIONS


class SkillNature(Enum):
Expand Down
2 changes: 1 addition & 1 deletion src/gui/position.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@

import pygame

Position = Union[pygame.Vector2, tuple[int, int]]
Position = Union[pygame.Vector2, tuple[float, float]]
2 changes: 1 addition & 1 deletion src/gui/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def show_fps(
def blit_alpha(
target: pygame.Surface,
source: pygame.Surface,
location: tuple[int, int],
location: Position,
opacity: int,
):
"""
Expand Down
45 changes: 21 additions & 24 deletions src/scenes/level_scene.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from src.game_entities.building import Building
from src.game_entities.character import Character
from src.game_entities.chest import Chest
from src.game_entities.consumable import Consumable
from src.game_entities.destroyable import DamageKind, Destroyable
from src.game_entities.door import Door
from src.game_entities.effect import Effect
Expand Down Expand Up @@ -252,7 +253,7 @@ def __init__(

# Storage of current selected entity
self.selected_player: Optional[Player] = None
self.selected_item: Optional[Item] = None
self.selected_item: Optional[Equipment | Consumable] = None
self.active_shop: Optional[Shop] = None

self.quit_request: bool = False
Expand All @@ -274,7 +275,6 @@ def __init__(
self.talk_sfx: Optional[pygame.mixer.Sound] = None
self.gold_sfx: Optional[pygame.mixer.Sound] = None


@property
def diary_entries_text_element_set(self):
"""
Expand Down Expand Up @@ -483,7 +483,7 @@ def update_state(self) -> bool:
Verify if victory or defeat conditions are met.
Handle next AI action if it's not player's turn.

Return the whether the game should be ended or not.
Return whether the game should be ended or not.
"""
if self.quit_request:
return True
Expand Down Expand Up @@ -738,7 +738,7 @@ def get_next_cases(self, position: Position) -> list[Optional[Entity]]:
return tiles_content

def get_possible_moves(
self, position: Position, max_moves: int
self, position: tuple, max_moves: int
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel like we are loosing some level of precision about the structure of position arg here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't make sense, must've been an overzealous mistake on my part.

) -> dict[Position, int]:
"""
Return all the possible moves with their distance from the starting position
Expand Down Expand Up @@ -769,7 +769,7 @@ def get_possible_moves(

def get_possible_attacks(
self,
possible_moves: Sequence[tuple[int, int]],
possible_moves: Sequence[Position],
reach: Sequence[int],
from_ally_side: bool,
) -> set[tuple[float, float]]:
Expand All @@ -781,7 +781,7 @@ def get_possible_attacks(
reach -- the reach of the attacking entity
from_ally_side -- a boolean indicating whether this is a friendly attack or not
"""
tiles: list[tuple[float, float]] = []
tiles: list = []

entities = list(self.entities.breakables)
if from_ally_side:
Expand Down Expand Up @@ -828,7 +828,7 @@ def is_tile_available(self, tile: Position) -> bool:

return entity_on_tile is None

def get_entity_on_tile(self, tile: Position) -> Optional[Entity]:
def get_entity_on_tile(self, tile: Position) -> Optional[Entity, Destroyable]:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destroyable is an Entity.
Why adding it to the typing while it's already included?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was a downstream complaint by a function expecting a Destroyable. Hopefully this will be clearer when I roll back to a less ambitious PR.

"""
Return the entity that is on the given tile if there is any

Expand All @@ -843,7 +843,7 @@ def get_entity_on_tile(self, tile: Position) -> Optional[Entity]:
return None

def determine_path_to(
self, destination_tile: Position, distance_for_tile: dict[tuple[int, int], int]
self, destination_tile: Position, distance_for_tile: dict[tuple, int]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it should be Position instead of tuple

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, I think I made this change before I realized there was a Position class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying it again, and it may be because Position is a Union with pygame.Vector2, which isn't hashable for the set function.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed I remember something about it, can be annoying...

) -> list[Position]:
"""
Return an ordered list of position that represent the path from one tile to another
Expand All @@ -853,11 +853,11 @@ def determine_path_to(
distance -- the distance between the starting tile and the destination
"""
path: list[Position] = [destination_tile]
current_tile: tuple[int, int] = tuple(destination_tile)
current_tile: tuple = tuple(destination_tile)
while distance_for_tile[current_tile] > 1:
# Check for neighbour cases
available_tiles: dict[tuple[int, int], int] = self.get_possible_moves(
tuple(current_tile), 1
available_tiles: dict[tuple, int] = self.get_possible_moves(
current_tile, 1
)
for tile in available_tiles:
if tile in distance_for_tile:
Expand All @@ -877,7 +877,7 @@ def distance_between_all(
entity -- the entity for which the distance from all other entities should be computed
all_other_entities -- all other entities for which the distance should be computed
"""
free_tiles_distance: dict[tuple[int, int], int] = self.get_possible_moves(
free_tiles_distance: dict[Position, int] = self.get_possible_moves(
tuple(entity.position),
(self.map["width"] * self.map["height"]) // (TILE_SIZE * TILE_SIZE),
)
Expand All @@ -899,8 +899,8 @@ def open_chest(self, actor: Character, chest: Chest) -> None:
Open a chest and send its content to the given character

Keyword arguments:
actor -- the character opening the chest
chest -- the chest that is being open
actor -- the character performing the action
chest -- the object that is being opened
"""
# Get object inside the chest
item = chest.open()
Expand Down Expand Up @@ -1142,7 +1142,7 @@ def interact(
elif isinstance(target, Character):
pygame.mixer.Sound.play(self.talk_sfx)

element_grid = target.talk(actor)
element_grid = target.talk()
self.menu_manager.open_menu(
InfoBox(
str(target),
Expand Down Expand Up @@ -1195,7 +1195,6 @@ def duel(
self,
attacker: Movable,
target: Destroyable,
attacker_allies: Sequence[Destroyable],
target_allies: Sequence[Destroyable],
kind: DamageKind,
) -> None:
Expand All @@ -1205,7 +1204,6 @@ def duel(
Keyword arguments:
attacker -- the entity that is making the attack
target -- the target of the attack
attacker_allies -- the allies of the attacker
target_allies -- the allies of the target
kind -- the nature of the damage that would be dealt
"""
Expand Down Expand Up @@ -1328,15 +1326,15 @@ def process_entity_action(self, entity: Movable, is_ally: bool) -> None:
entity -- the entity for which the action should be computed
is_ally -- a boolean indicating if the entity is an ally or not
"""
possible_moves: dict[tuple[int, int], int] = self.get_possible_moves(
possible_moves: dict[Position, int] = self.get_possible_moves(
tuple(entity.position), entity.max_moves
)
targets: Sequence[Movable] = (
self.entities.foes if is_ally else self.players + self.entities.allies
)
allies: Sequence[Movable] = (
self.players + self.entities.allies if is_ally else self.entities.foes
)
# allies: Sequence[Movable] = (
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it's something you planned to fix later on because it's still only a draft PR (I know), but I'm putting this comment as a reminder

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allies doesn't actually get used by the duel function on line 1194. Seems like something that might get implemented in the future, so I didn't want to get rid of it entirely.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah feel free to remove not used stuff.

We are using Git for a reason: if some later time we need these changes (because of a feature related to allies) then we could just get back what was removed by the commit.

Commented out code is a code smell, and is harmless to remove when having a good versioning system tool.

# self.players + self.entities.allies if is_ally else self.entities.foes
# )
tile: Optional[Position] = entity.act(
possible_moves, self.distance_between_all(entity, targets)
)
Expand All @@ -1349,8 +1347,8 @@ def process_entity_action(self, entity: Movable, is_ally: bool) -> None:
entity.set_move(path)
else:
# Entity choose to attack the entity on the tile
entity_attacked = self.get_entity_on_tile(tile)
self.duel(entity, entity_attacked, allies, targets, entity.attack_kind)
entity_attacked: Destroyable = self.get_entity_on_tile(tile)
self.duel(entity, entity_attacked, targets, entity.attack_kind)
entity.end_turn()

def interact_item_shop(self, item: Item, item_button: Button) -> None:
Expand Down Expand Up @@ -2088,7 +2086,6 @@ def left_click(self, position: Position) -> None:
self.duel(
self.selected_player,
entity,
self.players + self.entities.allies,
self.entities.foes,
self.selected_player.attack_kind,
)
Expand Down
Loading
Loading