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

Add Circle move()/move_ip() #2561

Merged
merged 8 commits into from
Jan 2, 2024

Conversation

itzpr3d4t0r
Copy link
Member

This PR adds the move/move_ip functions to the Circle object. It also adds the possibility of doing equality/inequality checks and an internal function for creating circle objects starting from three values.

Credits

Geometry Project:
For code, docs and tests:
@novialriptide @Emc2356 @itzpr3d4t0r @ScriptLineStudios @avaxar @Matiiss @newpaxonian @maqa41 @blankRiot96
Also thanks to @Starbuck5 for kickstarting the idea and the occasional help!

Functionality added in this PR

Co-authored-by: Emc2356 <63981925+emc2356@users.noreply.github.com>
Co-authored-by: NovialRiptide <35881688+novialriptide@users.noreply.github.com>
Co-authored-by: ScriptLineStudios <scriptlinestudios@protonmail.com>
Co-authored-by: Avaxar <44055981+avaxar@users.noreply.github.com>
Co-authored-by: maqa41 <amehebbet41@gmail.com>
@itzpr3d4t0r itzpr3d4t0r added New API This pull request may need extra debate as it adds a new class or function to pygame geometry pygame.geometry labels Nov 11, 2023
@itzpr3d4t0r itzpr3d4t0r requested a review from a team as a code owner November 11, 2023 13:46
@itzpr3d4t0r itzpr3d4t0r mentioned this pull request Nov 11, 2023
87 tasks
@itzpr3d4t0r itzpr3d4t0r changed the title Add Circle move()/move_ip() Add Circle move()/move_ip() Nov 11, 2023
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 👍

src_c/circle.c 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.

looks like this PR has conflicts now

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.

Left another nitpick about richcompare, though I'm happy with move[_ip] so I will go ahead and approve this, thanks for the PR 🎉

src_c/circle.c Outdated
{
pgCircleBase c1, c2;
if (!pgCircle_FromObject(self, &c1) || !pgCircle_FromObject(other, &c2)) {
Py_RETURN_NOTIMPLEMENTED;
Copy link
Member

@ankith26 ankith26 Jan 2, 2024

Choose a reason for hiding this comment

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

The general python convention for implementing comparison of non-matching types is:
== should be False, != should be True and all other operations should raise the not implemented error

I know pygame doesn't stick to this convention in a lot of places, but we can certainly consider this for new code

@itzpr3d4t0r itzpr3d4t0r added this to the 2.5.0 milestone Jan 2, 2024
@itzpr3d4t0r itzpr3d4t0r merged commit ea33a4f into pygame-community:main Jan 2, 2024
29 of 30 checks passed
@itzpr3d4t0r itzpr3d4t0r deleted the circle-move-moveip branch January 3, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
geometry pygame.geometry New API This pull request may need extra debate as it adds a new class or function to pygame
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants