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

Rect() and FRect() initialization with no arguments #2655

Merged
merged 7 commits into from
Jan 28, 2024

Conversation

damusss
Copy link
Contributor

@damusss damusss commented Jan 4, 2024

I've added 3 lines of code to the C src to allow Rect and FRect to be made by passing no arguments to them, which will result in a zero rect (x, y, w, h = 0).
The check is so little it should not impact performance at all.

Why did i do this?

  • Code speed and quantity: this is an insignificant point but it takes less time/you type less to do pygame.Rect() rather than pygame.Rect(0, 0, 0, 0)
  • It feels like it should be possible + prettier to see
  • Consistency: the main point: why can i do pygame.Vector2() and i can't do pygame.Rect()? Well, now you do
  • Same performace. I am very confident a little sequence length check on the args won't impact performance at all

Other than the C code i modified:

  • The _GenericRect class in the rect.pyi stub to include parameterless __init__
  • Added some lines of documentation to explain the new behaviour (i'm not the best at documenting, if you'd prefer something different tell me)
  • Added a test in rect_test.py to verify the correct initialization of the rect

@damusss damusss requested a review from a team as a code owner January 4, 2024 14:11
src_c/rect_impl.h Outdated Show resolved Hide resolved
@novialriptide
Copy link
Member

The code looks fine, but is there any use case for this?

@damusss
Copy link
Contributor Author

damusss commented Jan 5, 2024

The code looks fine, but is there any use case for this? (@novialriptide)

There are some, i think. Maybe you want to create a rectangle and assign it's values later, just like you'd do with a Vector2.
In my UI library you could benefit from this alot. Each element requires a Rect for positioning and sizing, but very often you just want for the element to resize automatically to fill the parent (specified in the style) so you pass an empty rectangle. I use this so much i even made the guiscript.ZeroR() function. Having it built in would be nice, and it would be also consistent as i can do that with pygame.Vector2 as mentioned above. temporary rects, dummy rects, rects made in the __init__/setup and then changed later etc could all be created without parameters. And since the check i'm adding is a simple tuple size check, i'm confident it won't impact performance so it also raises the question "why not?".

src_c/rect_impl.h Outdated Show resolved Hide resolved
test/rect_test.py Show resolved Hide resolved
docs/reST/ref/rect.rst Outdated Show resolved Hide resolved
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR 🎉

I agree with the reasoning to add this API, might not be a huge deal, but does no harm so why not.

I also see that __new__ inits the fields to 0 already, so there's no issue of uninitialised memory at any point

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and the API makes sense. 👍

@itzpr3d4t0r itzpr3d4t0r merged commit 35e34e4 into pygame-community:main Jan 28, 2024
30 checks passed
@itzpr3d4t0r itzpr3d4t0r added the rect pygame.rect label Jan 28, 2024
@itzpr3d4t0r itzpr3d4t0r added this to the 2.5.0 milestone Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rect pygame.rect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants