Typehint warnings #86
Conversation
| @@ -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 | |||
There was a problem hiding this comment.
feel like we are loosing some level of precision about the structure of position arg here
There was a problem hiding this comment.
This change doesn't make sense, must've been an overzealous mistake on my part.
| @@ -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]: | |||
There was a problem hiding this comment.
Destroyable is an Entity.
Why adding it to the typing while it's already included?
There was a problem hiding this comment.
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.
| @@ -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] | |||
There was a problem hiding this comment.
Maybe it should be Position instead of tuple
There was a problem hiding this comment.
That makes sense, I think I made this change before I realized there was a Position class.
There was a problem hiding this comment.
Trying it again, and it may be because Position is a Union with pygame.Vector2, which isn't hashable for the set function.
There was a problem hiding this comment.
Indeed I remember something about it, can be annoying...
| allies: Sequence[Movable] = ( | ||
| self.players + self.entities.allies if is_ally else self.entities.foes | ||
| ) | ||
| # allies: Sequence[Movable] = ( |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| @@ -71,19 +71,19 @@ def __init__(self, screen: pygame.Surface) -> None: | |||
| self.level: Optional[LevelScene] = None | |||
| self.exit: QuitActionKind = QuitActionKind.CONTINUE | |||
|
|
|||
| StartScene.load_options() | |||
| self.options_file = None | |||
| self.load_options() | |||
There was a problem hiding this comment.
I think you need to rebase your branch.
There was a problem hiding this comment.
I'll start from scratch and just change a reasonable number of scripts at a time. I think I started with level_scene.py, and the dependencies spiraled quicker than I expected.
Fixed linter and type hint warnings without changing functionality, mostly for PyCharm compatibility.