Tutorial 2025: Update to TCOD 19.3 #60
Conversation
There was a problem hiding this comment.
All the linters of this repo have been ignored, causing several blatantly incorrect changes to be mode to the source (broken type-hints and incorrect whitespace). The linter configuration is also outdated and should be bumped to target Python 3.11 or later.
Always ensure that Ruff and Mypy are happy with the source before committing changes.
Even if I approved this then it'd still have to coincide with updates to the website repository before it can be merged. Unless @TStand90 comments on this then that isn't happening.
|
Thanks for the quick review. I've retraced the tutorial path and hit every step with black / ruff / mypy, and added all missing type annotations. Let's see if github's run of the linter agrees... |
|
It appears that you've attempted to move files, but they've been copied instead. |
|
Also your linter scripts do not cover If I were setting up automated linter scripts then I'd use pre-commit. |
|
Were these bash scripts intended to be committed? |
jmccardle
force-pushed
the
master
branch
2 times, most recently
from
7d56ce1 to
93d7464
Compare
|
alright, think I've got it now -
I did eventually enhance my routine with |
Feel free to commit the file. |
|
this pass removed tcod.image in favor of directly using pillow I have some thoughts on improving the aesthetics of how modules are imported, I'm not entirely happy with what's here, but I can also feel that I'm running out of inertia. At the very least my fork is something I can share to help people using the tutorial get their code to execute with modern tcod and without the refactoring steps of tutorials 6, 8, and 10. I think the way forward is with evaluating larger rewrites, like reordering lessons, going all-in on ECS, etc |
The current file changes show the opposite.
I also have options on this. My current concern is that abstract classes should never be in the same module as their subclasses like they are now (main offenders being state and action base classes which are not even abstract when they should be). Doing this would cut down on cyclic imports (which I'd attempt to ban in the tutorial source code). I'd also like to hear your specific thoughts on imports.
Great job. I know that isn't easy.
I've had thoughts on this too, and a lot of time to think about this in general. I wouldn't mind collaborating on a new tutorial if you're interested. |
| self.width, self.height = width, height | ||
| self.entities = set(entities) | ||
| self.tiles = np.full((width, height), fill_value=tile_types.wall, order="F") | ||
| self.entities: Set[game.entity.Entity] = set(entities) |
There was a problem hiding this comment.
Possible redundant type-hint here. set(entities) already defines this hint.
Even if you wanted to explicitly hint this, typing.Set is known to be deprecated, replaced by collections.abc.Set.
Yikes, my bad. This was changed in part 10 then immediately overwritten in part 11, probably a clumsy
Absolutely, I think that would be a great deal of fun. I've been a consumer of libtcod for some time now and it's been exciting to work with you just on these little updates. |
I attempted to make each tutorial a minimal change over the 2021 version.