Add (F)Rect.collidecircle()
#2886
Conversation
|
I think a reasonable argument can be made, regardless of the technical difficulties, that it just makes sense to have all our geometry shapes (Rect, FRect & Circle) under one submodule long term. |
Exactly, I totally agree. geometry is not geometry if there aren't all geometries in it. |
There was a problem hiding this comment.
This works in my local testing, and I actually generally approve of moving Rect & FRect under geometry with a little python shim layer for backwards compatibility.
I don't know why we are keeping import_pygame_rect() around but that's not essential to this PR, so I will approve anyway.
|
|
||
| #define import_pygame_rect() IMPORT_PYGAME_MODULE(rect) |
There was a problem hiding this comment.
Should we consider just replacing all calls to import_pygame_rect() with import_pygame_geometry() and removing import_pygame_rect() ? It is internal API only so we are likely only confusing ourselves in the future by keeping import_pygame_rect() around and just having it import geometry.
There was a problem hiding this comment.
I didn't do that as I didn't know if doing that would cause too many internal changes, but I initially wanted to rename it, and I can do it if it's needed. Update me if you think I should change it, maybe based on some other contributor's opinion.
There was a problem hiding this comment.
It's not essential to this PR IMHO, I guess we'll see how everyone else feels about moving Rect/Frect and shimming.
itzpr3d4t0r
added
the
New API
| @@ -113,3 +122,4 @@ class Circle: | |||
| def as_frect(self) -> FRect: ... | |||
| def __copy__(self) -> Circle: ... | |||
| copy = __copy__ | |||
|
|
|||
|
Needless to say I'm on the same boat with @Damus666 on this one. I think this move makes sense and it's actually an elegant solution which requires minimal code change on our end and has 0 impact to the end user. It might even solve some people's questions like: "why can't I import Rect/Frect from geometry? Isn't it a shape?" in the future. |
There was a problem hiding this comment.
Aside from a docs correction this LGTM on the pygame-ce side. On the geometry side i got this error:
I suppose we'll need to update our pygame.h file in geometry, so it's not an issue.
After my approval the PR could be merged but I think it's better to hear from someone else on the topic like @ankith26 or @Starbuck5.
There was a problem hiding this comment.
Essentially, this seems like the first change where something is being added to "stable API" to support something in the "experimental API" section.
These changes look somewhat risky, and personally, I would like to hold it back for a while, till we can claim pygame.geometry as "stable" as a whole
As we near the completion of porting
Circle's functionality, we're preparing to implement collision methods forRect, such as.collidecircle()and.collideswith().Though this might seem straightforward, it's more complex than it appears. PR #2731 by @itzpr3d4t0r attempted to reorganize our include files to facilitate this, but it wasn't entirely successful. While it allowed for partial implementation of these functionalities, it didn't fully resolve the issue. The current include structure, combined with Rect's, prevents us from recognizing when a Circle object (or, in the future, a Line or Polygon) is passed as an argument.
This is a significant problem because it undermines the core value of the pygame-geometry project: having interconnected shapes with common methods, enabling method calls across different object types.
@itzpr3d4t0r and I explored various approaches in C, but none were successful because we need to include the rect module inside geometry and vice versa. The only viable solution, short of completely rewriting the C code for Rect/Frect and Circle, is to solve the problem using Python. By moving Rect/Frect to the geometry module, we can call methods from any shape.
This PR achieves that while maintaining 100% backward compatibility. You can still use
pygame.Rectorpygame.rect.Rectas before, but now you can also usepygame.geometry.Rect. This change does not affect the end user but allows us to properly implement all of pygame-geometry's features.