-
-
Notifications
You must be signed in to change notification settings - Fork 158
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
geometry
module
#2267
Comments
Few questions I have related to the api, the implementation and how it will be moved to the main repo:
And finally... should we add this at all? I know you all worked on this hard, I do not want to discredit anyone... but you kind of just came here and opened an issue saying "we will do this" and a pr. As far as I know there was no recent discussion about it, not since creating the pygame-geometry repo, which was now a year(ish) ago. Not saying I want to 100% deny all this, but an earlier heads-up, more discussion and coordination would help a lot in making this more frictionless in my opinion. And merging it into here would mean we have to support it no matter what. Possibly putting in new api specifically for new shapes, people maybe asking for more shapes for some reason, functions having another path for if a shape was passed in, etc... While if we don't merge it here, we could just make it a "strongly recommended" library. Kind of like sdl and all the other libraries do it (sdl_ttf, sdl_image, sdl_mixer, for example). They are maintained by the same people (for some/most of them), but are not part of the core. But these are my 2 cents anyway. |
First of all thanks for commenting. Unfortunately for me I have this bad habit of opening PRs very fast with little notice, sorry for that.
These are all excellent ideas. Adding these shapes to pygame can enhance multiple functionalities while maintaining backward compatibility. I believe, however, that potential futher enhancements to existing functionalities should be addressed when those functionalities are actually proposed (with an Issue). The main goal of porting the pygame.geometry project is to introduce new classes, methods, and attributes, which have already been successfully implemented. So aside from stuff that we can't change because it would break back compat, there isn't much else to discuss about potential future additions in this specific moment in time.
I'm neutral about this.
I'm not exactly sure about the process yet. For now I'm just taking code from the geometry project and trying to adapt it to the pygame repo without too much change. But yeah co-authoring will be much welcome. Since many people contributed to pygame-geometry i believe it's unlikely for every single person to open their separate PR to add their own changes, so there must be someone who will do that for them.
The plan is to slowly implement functionality, hopefully one shape at a time and with careful review and considerations.
Yes of course! Who thinks to copy something 100% from somewhere to an open source repo without changes?
The shapes all use double precision numbers to represent their attributes. We made this choice because it would be much more precise (especially after tons of transformations) and because python itself uses doubles by default, so it would be easier to implement since there would be less indirection.
I'm not the one who is going to decide whether we should add this. But this story isn't new at all as you stated, we talked about this in the contributing chat months ago, we opened an issue on the old pygame repo (here: pygame/pygame#3444) and it was much appreciated and everyone we talked to seemed enthusiastic about these additions. I opened the PR and went gun blazing partly because that's the only way to kickstart discussion and because it's my habit (a bad one but still a habit).
I personally don't like the idea of keeping this as a separate library. Mainly because this should be functionality that's easy to access for new users, especially because of the power that it has. Having it in a separate library means you have to download a separate package, and it also means that it should still be supported somehow, so someone has to do it anyway if we go down that path. Everyone i talked to seemed enthusiastic about these functionalities, so if we are scared to make it easily accessible to the widest amount of people we are just not doing the community's best. |
I really like this addition, and I myself was hoping that something similar will be implemented. |
I really like the idea of a new module, this will help us a lot. Two consequences however in my opinion of this new module :
Again, thank you very much for your hard work ! |
Good idea. Right now the hitboxes are in a @itzpr3d4t0r, I'd like to see a written out interface that you and the team plan on following for the shapes. I'm interested to see what changes you would make to Rects to make it follow this interface. Like a generic And as for the current PR, I'd like it to be a private module. Warning about compat at top, Circle not in default namespace, maybe even module hidden from docs, full 9 yards. |
By co-athoring I mean the github supported way which should be done when someone makes a pr that adds someone else's work, even if it is a small change. I would even go as far as linking the section of the code added with blame on (like this for example), or the original commit that added it if it is a single commit (like this) (however, your idea is good too, let people who opened the pr on the geometry repo open it here as well). I want people to be properly credited, so I will try to be strict about this when reviewing, and I urge others to do the same as well.
That is good to hear. This also means then you are aware that it will take a lot of time as well, which is good. And since we are approaching it this way, making the module private and not advertising it would make more sense imo. Not until it is completely done.
I meant API changes, that are changed when opening the pr. Not after review, that are suggested. I am asking because then why not commit to the geometry module first and then copy them over? As I said before, I would like the geometry repo to be a place where one can see the proper credits of people, so this makes more sense in my eyes. This would also make merging into the pygame repo much faster, since we don't have to decide on the API itself, just the code.
Yes, I stated that it was discussed to some degree (again, a year ago now), but even that was not that thorough to be honest. Yes it is a start, but since then, you implemented the api. Where is the discussion about that? Because the only place I see it is in a thread in the contributing channel in discord with like 20 people in it. Did you ask the opinion of people (the everyone you are talking about here) about that? Were they still enthusiastic? Would they change add or remove anything? Saying "we are just not doing the community's best" while not asking them more in-depth about the very thing you are talking about sounds off to me. This would also reduce the amount of API change that needs to be done while merging into pygame
This is not the only way to kickstart discussion, and is a bad attitude. Ask people to share their opinion, share easy to grasp and concrete plans and ideas, promote it. Yes, it is a way, but this is not the way it should be done. It just puts pressure on maintainers, adds another pr to the list that will not move for a long time and might misdirect people from caring about the planning to the code.
I mean, if someone can get pygame(-ce) then they can also get the extra geometry module without much effort. And while sure we would have to keep the geometry module alive, it would reduce the amount of maintenance the team has to do since we would not have to integrate all of geometry to other parts of the code.
|
This is why this is going to be an experimental, hidden, unstable (all the good stuff) module at first. Worst case scenario the momentum falls off and enters the zombie purgatory world of _sprite and _sdl2.audio.
After pulling some teeth on discord I believe I saw an image of an API interface out on the pygame-geometry channel. If that exists, could you please post it here. |
This is the interface, we are still missing implementations for:
There are also a couple of uncertainties:
|
API suggestions looking this over Bring Bring |
I'm pleased to inform you that the
pygame-geometry
library has started being ported to thepygame-ce
repository.The primary goal is to enhance the functionality of the library by introducing several new types of shapes, while retaining the familiar features found in pygame's
Rect
orFrect
classes.The main portion of the functionality consists of the following shape classes:
Circle
Line
Polygon
Each shape includes the following:
Please note that the current pygame-geometry codebase may contain functionality that will not be included in the port or may undergo significant modifications.
If you are interested in exploring the complete codebase for the project or help with the port, you can find it here:
Integration Progress
The integration is happening in phases, starting with Circle and later Line and Polygon. Each shape will then be slowly built upon piece by piece.
At the moment the Line object is being actively ported
geometry
module,Circle
base #2268pg_DoubleFromObj
/pg_TwoDoublesFromObj
C api:geometry
module,Circle
base #2268Circle
Circle
object:geometry
module,Circle
base #2268Attributes
x
:geometry
module,Circle
base #2268y
:geometry
module,Circle
base #2268r
:geometry
module,Circle
base #2268r_sqr
: Added missing Circle attrs #2519diameter
/d
: Added missing Circle attrs #2519center
: Added missing Circle attrs #2519area
: Added missing Circle attrs #2519circumference
: Added missing Circle attrs #2519top
/bottom
/left
/right
: Added additional circle attributes #3127Methods
copy()
:geometry
module,Circle
base #2268repr
/str
/print
:geometry
module,Circle
base #2268move
/move_ip
: Add Circlemove()
/move_ip()
#2561update
: Add Circleupdate()
#2562as_rect
/as_frect
: Add Circleas_rect()
/as_frect()
#2634rotate / rotate_ip
: Add Circlerotate()
/rotate_ip()
#2662collidepoint
: Add Circlecollidepoint()
#2536collidecircle
: Add Circlecollidecircle()
#2540colliderect
: Add Circlecolliderect()
#2560collideswith
: Add Circlecollideswith()
#2661contains
: Add Circlecontains()
#2791collidelist / collidelistall
: AddCircle.collidelist/collidelistall()
#2880intersect
: AddCircle.intersect()
#3071collideline
collidepolygon
Line
Line
object: Addedgeometry.Line
base #3131Attributes
ax
,ay
,bx
,by
Addedgeometry.Line
base #3131a
,b
Addedgeometry.Line
base #3131center
: Add moreLine
attributes #3268centerx
,centery
: Add moreLine
attributes #3268length
: AddedLine
methods andlength
attribute #3179angle
: Add moreLine
attributes #3268slope
: Add moreLine
attributes #3268Methods
move
/move_ip
: AddedLine
methods andlength
attribute #3179rotate
/rotate_ip
scale
/scale_ip
: AddedLine
methods andlength
attribute #3179flip_ab
/flip_ab_ip
: AddedLine
methods andlength
attribute #3179update
: AddedLine
methods andlength
attribute #3179copy
: Addedgeometry.Line
base #3131collidepoint
collidecircle
collideline
colliderect
collidepolygon
collideswith
as_circle
as_rect
is_parallel
is_perpendicular
at
as_segments
as_points
collidelist / collidelistall
intersect
Polygon
Polygon
objectAttributes
vertices
verts_num
center
centerx
,centery
perimeter
area
Methods
move
/move_ip
copy
collidepoint
collideline
collidecircle
insert_vertex
remove_vertex
pop_vertex
is_convex
scale
/ scale_ipas_segments
as_rect
rotate
/rotate_ip
collidelist / collidelistall
intersect
Rect & Frect
Methods
collidecircle
: Add(F)Rect.collidecircle()
#2886collideline
collidepolygon
collideswith
as_polygon
Other
regular_polygon
raycast
multiraycast
Circle
into draw.circleLine
into draw.linePolygon
into draw.polygonThe text was updated successfully, but these errors were encountered: