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

Enhancement: Add support for rotation offsets for sprites (anchor) #1102

Open
pushfoo opened this issue Mar 11, 2022 · 7 comments
Open

Enhancement: Add support for rotation offsets for sprites (anchor) #1102

pushfoo opened this issue Mar 11, 2022 · 7 comments
Milestone

Comments

@pushfoo
Copy link
Member

pushfoo commented Mar 11, 2022

Enhancement request:

What should be added/changed?

tl;dr: Add rotation angle offset support for sprites

This would take the form of angle_offset and radians_offset to match the respective attributes of Sprite. To avoid conflict between the two settings, the default values could be None, raising an exception if both receive non-None values.

Different Sprite subclasses could also have different default values to make use of sprites as they're found.

The implementation could be one of the following:

  • Extra parameters for load_texture or Texture, with pass-through versions of the args in Sprite's constructor
  • Class variable
  • Instance variable set from constructor argument + property
  • Mix of the above

Each option seems to potential have advantages as well as disadvantages.

What would it help with?

tl;dr: Increase development speed

Although flipped_horizontally and flipped_vertically are supported as arguments to Sprite, these don't allow for rotation that would be useful for non-symmetrical sprites.

Adding rotation support would:

User Story Example

Alex is a student working on a top-down game on a somewhat underpowered computer. He finds a good image of a vehicle, but it points the wrong way when loaded into arcade. Alex passes -90 to angle_offset to immediately make the image load correctly. No time is wasted on waiting for an image editor to load.

Possible advantages of different implementations

Option Advantage
Extra parameters to load_texture or Texture, mirrored in Sprite's constructor Improves texture loading flexibility in general
Class variable Allows introducing users to the idea of a class variable in Python
Instance variables Could make creating some interesting game behavior easier
A mix Flexibility

Class variables could be confusing to beginners, so they might not be the best default interface for this feature.

@einarf
Copy link
Member

einarf commented Mar 12, 2022

I think support for loading a image rotated is the simplest path there. We're also trying to clean up sprite initialization so there could also be room for a different classmethod for creation a sprite from a rotated texture.

The offset angle could possibly only apply to one texture for that sprite so the simples way is to actiually fix the original image. If multiple rotated versions pack as a single or multiple images in the internal atlas is all up to the atlas itself.

@pvcraven
Copy link
Member

I like this as a possible solution. I'm not really wild about adding stuff to our overly-large Sprite class unless there's a clear need. This would confine it to loading of textures.

@einarf
Copy link
Member

einarf commented Mar 21, 2022

What we could do is making some tutorial with some examples on how to extend the Sprite and add functionality. Keeping the base sprite class simple has so many advantages.

@pushfoo
Copy link
Member Author

pushfoo commented Apr 7, 2022

I'm in agreement on a minimal implementation that only adds a parameter to load_texture. It would speed up implementation and help simplify the code for #1176.

@pushfoo
Copy link
Member Author

pushfoo commented Apr 7, 2022

I think there's also room for expanding on this to allow a partial set of top-down set of sprites to be turned into a full complement of textures, but that needs additional consideration and should be split into another ticket.

@pushfoo
Copy link
Member Author

pushfoo commented Apr 7, 2022

Per discord discussion with @einarf, this should probably be delayed until it can be handled using improvements from #1154. Rushing this feature with a naive approach of pillow rotation has the potential to indirectly waste significant amounts of memory and break code using the current texture cache naming scheme.

We should also limit rotations to multiples of 90 degrees to simplify code. It's the most common use case for rotating sprite files.

@gran4
Copy link
Contributor

gran4 commented May 6, 2023

We should also limit rotations to multiples of 90 degrees to simplify code. It's the most common use case for rotating sprite files.

Have mixin to add stuff like this?

@einarf einarf changed the title Enhancement: Add support for rotation offsets for sprites Enhancement: Add support for rotation offsets for sprites (anchor) Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants