-
-
Notifications
You must be signed in to change notification settings - Fork 158
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
sprite group classes > support source_rect functionality #3254
Comments
If one is using a sprite sheet, wouldn't it be easier to just cut out the part of the surface that should be part of the sprite? You can use .subsurface for it without even using any additional memory. |
sure, was just an example. Is the source_rect functionality not useful to have built in for things like that and should only remain in DirtySprite for backwards compatibility reasons? |
Separately, for sprite sheet animations, is it better to just change source_rect over time or keep reassigning |
It seems most straightforward to keep re-assigning |
Okay, does it make sense to support source_rect dynamically in the other draw functions at all, or is this basically just functionality we don't need and it only needs to remain available for DirtySprite to not break backwards compatibility? If the former I can make a different PR, if the latter, please go ahead and close this issue and I'll remove the PR. |
Currently the
LayeredDirty
class blits sprites taking into accountDirtySprite
'ssource_rect
attribute. I would like all of the groups to take into account anySprite
'ssource_rect
attribute.pros: I think it would be useful for all groups to use a
source_rect
value if present, to enable easy e.g. sprite sheet functionality.cons: checking another attribute per sprite per draw, I don't think this will have a big performance impact but maybe that's a particular concern.
Questions about approach:
source_rect
(defaulted to None) on theSprite
class so we can assume it exists, or should I not assume it exists and get it withgetattr(spr, 'source_rect', None)
or similar?Thanks for reading! I'd love to get feedback.
The text was updated successfully, but these errors were encountered: