Skip to content

Fix Camera2D render target size bug & improve durability #2050

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

Merged
merged 12 commits into from
Apr 6, 2024

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
7 of 8 checks passed
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