Skip to content

Conversation

@pushfoo
Copy link
Member

@pushfoo pushfoo commented Apr 6, 2024

Changes

  1. Fix apparent bug where render target sizes are ignored by Camera2D
  2. Use more durable keyword argument passing for dataclass arguments
  3. Make Camera2D.from_raw_data inheritance-safe
  4. Correct Camera2D.viewport's docstring
  5. Add tests for the changes above
  6. Move the orthographic projector tests to an appropriately named file

Follow-up Work

Vec types

Once #2043 is merged, we should probably use its Vec* types liberally

Fancy / Named Tuple types?

Would NamedTuple or similar be better for internal viewport representation? Benefits I can see:

  1. Provides named access which is cleaner and more legible
  2. Named access may be faster for named props according to Ben's recent benchmarks for pyglet vectors (needs verification)

LRBT and similar types might be useful.

Validation?

Currently we don't do any pyglet-style implicit unpacks or length checks. Maybe we should?

pushfoo added 7 commits April 6, 2024 07:16
* Use final render_target.size instead of window size

* Clean up . accesses for shorter code + tiny speed boost
* Get render_target earlier

* Use render_target.size to define width and height

* Make code cleaner +_ a tiny bit faster with less . access
* Make it a class method with a typing_extensions.Self return type

* Add a test for subclassing safety
@pushfoo
Copy link
Member Author

pushfoo commented Apr 6, 2024

The remaining type issues in the GitHub based tests all appear to be GUI stuff.

@pushfoo
Copy link
Member Author

pushfoo commented Apr 6, 2024

TL;DR: Changes didn't break anything, it's just the GUI pyright issues again

The changes are responses to Discord feedback from DragonMoffon. GitHub based tests still report the same GUI breakage as before, and those are unrelated to this PR.

@einarf einarf merged commit e2ef4a9 into pythonarcade:development Apr 6, 2024
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.

2 participants