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

Reorganize geometry internals #2731

Merged

Conversation

itzpr3d4t0r
Copy link
Member

@itzpr3d4t0r itzpr3d4t0r commented Feb 26, 2024

This is a preparation PR to enable us to have functions like Rect/Frect.collidecircle() and in general interoperability with shapes. Specifically it fixes a circular import we had going on.

What this PR really does at a high level is moving some functions that need to get used by rects/circles/lines/polygons into a separate header file so that it can be included everywhere without circular imports.

Deleted:

  • collisions .h/.c

Added:

  • geometry_common.c/.h (containing common collision functions and PyObject->Shape conversion functions.)

Simplified:

  • Some unnecessary imports

@itzpr3d4t0r itzpr3d4t0r added Code quality/robustness Code quality and resilience to changes geometry pygame.geometry labels Feb 26, 2024
@itzpr3d4t0r itzpr3d4t0r requested a review from a team as a code owner February 26, 2024 16:22
@itzpr3d4t0r itzpr3d4t0r added this to the 2.5.0 milestone Feb 27, 2024
Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

I know this is just a bunch of moving stuff around, but I found some nitpicky things to comment on anyway lol. IMO none of these are a pull blocker though, so I'm approving

src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
src_c/geometry_utils.c Outdated Show resolved Hide resolved
@itzpr3d4t0r itzpr3d4t0r force-pushed the reorganize-geometry-internals branch from e762bab to 300ad20 Compare March 12, 2024 12:45
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 🎈

@itzpr3d4t0r itzpr3d4t0r merged commit 5a4acb6 into pygame-community:main Mar 14, 2024
29 of 30 checks passed
@itzpr3d4t0r itzpr3d4t0r deleted the reorganize-geometry-internals branch March 16, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality/robustness Code quality and resilience to changes geometry pygame.geometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants