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

Name scale properties on Sprites to things that make sense #2021

Open
wants to merge 8 commits into
base: development
Choose a base branch
from

Conversation

DigiDuncan
Copy link
Contributor

@DigiDuncan DigiDuncan commented Mar 13, 2024

What does it do?

  • Allows setting scale to a float for backwards compatible codefeel/easy proportional scaling
  • Getting scale returns a tuple[float, float]
  • Renames scale -> scale_x
  • Renames scale_xy -> scale
  • Adds scale_y

Why should this be done?

  • The current implementation lies to you. scale as it is returns scale_x, which is incorrect if x/y are scaled independently.
  • There is precedent. Sprite currently returns position¹ as a 2-tuple, and center_x and center_y as floats.
  • Learning wrong is worse than not learning. The current implementation is more confusing for children in the long run. Most game libraries represent scale in both dimensions, and Arcade should aim to provide transferable knowledge.
  • It's confusing for a developer. If you are a games dev, understanding why .scale returns x, scale_y returns y, scale_xy returns a tuple, but setting scale sets both x and y, is very confusing and makes code harder to read and reeks havoc on codefeel.

¹I'd argue position should be center as well, but that's for another day.

@pushfoo
Copy link
Member

pushfoo commented Mar 15, 2024

TL;DR: This will be a non-breaking change & efficiency upgrade with pyglet 2.1, which seems to be coming sooner than expected.

Benefits:

  1. sprite_instance.scale *= 2.0 will work with scalars as it does now
  2. sprite_instance.scale *= 2.0, 3.0:

Steps required:

  • Bump to pyglet 2.1 (Switch to Pyglet 2.1 or dev releases #2043)
  • Make BasicSprite.scale return pyglet 2.1's tuple-based Vec2s
  • Improve doc to use/review position to teach tuples, then lead into scale
    1. cute diagram with scale x and scale y, stretching a snake image
    2. scale example with interactive GUI sliders to scale sprite
    3. add links to healthbar examples
  • Revert any example temp fixes to pass CI / typing

@einarf einarf self-requested a review March 25, 2024 23:12
@einarf einarf added this to the 3.0 mandatory milestone Mar 25, 2024
@pushfoo pushfoo mentioned this pull request Apr 4, 2024
8 tasks
@einarf
Copy link
Member

einarf commented Apr 16, 2024

What about the initializer? Should scale still only be a float there?

@pushfoo
Copy link
Member

pushfoo commented Apr 16, 2024

Imo, it could be either without an isinstance check:

if hasattr(scale, "__len__"):
    self._scale = Vec2(*scale)
else:
    self._scale = Vec2(scale, scale)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants