Revamp Assets #306
Merged
Revamp Assets #306
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pathunstrom
reviewed
Jul 11, 2019
pathunstrom
reviewed
Jul 11, 2019
pathunstrom
reviewed
Jul 11, 2019
| if image_name not in self.resources: | ||
| self.register_renderable(game_object) | ||
| if isinstance(image, str): | ||
| logger.warn(f"Using string resources is deprecated, use ppb.Image instead. Got {image!r}") |
There was a problem hiding this comment.
Only thing about this is that it's extremely verbose (every frame).
There was a problem hiding this comment.
Could throw in a tracking variable and only throw it once? (Or once per object?) Would lru_cache wrapped around a print statement make it run once per object passed?
There was a problem hiding this comment.
Yeah, all of those things. It's just annoying, though?
pathunstrom
reviewed
Jul 11, 2019
| import io | ||
|
|
||
|
|
||
| class Image(Asset): |
There was a problem hiding this comment.
Dunno on adding new things to systems.__init__. If we're making changes to the renderer and adding new things they should move to its own file.
There was a problem hiding this comment.
Yeah, I was wondering if we wanted to move the renderer to its own module.
There was a problem hiding this comment.
It's one of the places we deal with pygame, so probably needs to go to systems.pg. (There's even argument for making pg.py into a subpackage of its own, but uh. . . not right now.)
Closed
|
This still needs tests. |
|
@pathunstrom I think this is ready for review & merge. |
This was referenced Jul 13, 2019
Merged
This was referenced Jul 13, 2019
pathunstrom
approved these changes
Jul 14, 2019
|
bors r+ |
Build succeeded
|
bors bot
added a commit
that referenced
this pull request
Jul 18, 2019
316: Move things, break loops r=pathunstrom a=astronouth7303 Grab a u-haul! 🚚 Generally moving things and reducing imports to improve the import loop situation * Break up "the machinery implementing a thing" and "a pile of things that use the thing" * Prefer module imports to individual imports, so any unavoidable loops don't wreck everything. Depends on #306 because I didn't feel like dealing with the merge conflict. No docs changes because this doesn't touch anything publicly documented. Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
bors bot
added a commit
that referenced
this pull request
Jul 18, 2019
316: Move things, break loops r=pathunstrom a=astronouth7303 Grab a u-haul! 🚚 Generally moving things and reducing imports to improve the import loop situation * Break up "the machinery implementing a thing" and "a pile of things that use the thing" * Prefer module imports to individual imports, so any unavoidable loops don't wreck everything. Depends on #306 because I didn't feel like dealing with the merge conflict. No docs changes because this doesn't touch anything publicly documented. Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
bors bot
added a commit
that referenced
this pull request
Jul 18, 2019
316: Move things, break loops r=pathunstrom a=astronouth7303 Grab a u-haul! 🚚 Generally moving things and reducing imports to improve the import loop situation * Break up "the machinery implementing a thing" and "a pile of things that use the thing" * Prefer module imports to individual imports, so any unavoidable loops don't wreck everything. Depends on #306 because I didn't feel like dealing with the merge conflict. No docs changes because this doesn't touch anything publicly documented. Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
bors bot
added a commit
that referenced
this pull request
Jul 18, 2019
316: Move things, break loops r=pathunstrom a=astronouth7303 Grab a u-haul! 🚚 Generally moving things and reducing imports to improve the import loop situation * Break up "the machinery implementing a thing" and "a pile of things that use the thing" * Prefer module imports to individual imports, so any unavoidable loops don't wreck everything. Depends on #306 because I didn't feel like dealing with the merge conflict. No docs changes because this doesn't touch anything publicly documented. Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
bors bot
added a commit
that referenced
this pull request
Jul 19, 2019
316: Move things, break loops r=pathunstrom a=astronouth7303 Grab a u-haul! 🚚 Generally moving things and reducing imports to improve the import loop situation * Break up "the machinery implementing a thing" and "a pile of things that use the thing" * Prefer module imports to individual imports, so any unavoidable loops don't wreck everything. Depends on #306 because I didn't feel like dealing with the merge conflict. No docs changes because this doesn't touch anything publicly documented. Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Totally revamp how PPB handles assets.
Depends on #313 because life was just easier that way.
Part of #147