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
Shape interface #2178
Shape interface #2178
Conversation
@MobiDevelop I must create new instances, as when the user requests a BoundingBox/Sphere/Vector3 and modifies it, upon next request the instance returned will be the same, thus containing modified values instead of the correct ones. |
Creating new instances each time these methods are called is going to set the GC on fire. You could have the user pass in an BoundingBox or Sphere (even though I still think it is weird to use 3D stuff for 2D shapes). That way it is up to the user to decide if they want to create new objects all the time. What's special about Polygon#getBoundingRectangle? |
I would obly merge if Shape is a tagging interface. getCenter would be ok,
|
I can also have the class keep final instances of BoundingBox/Sphere/Vector3 and have their values update each time the Shape's getter methods are called. Of course, I could also let the user choose whether to pass their own objects to write values to. As I said earlier, I chose 3D bounding shapes for compatibility, but it's not vital. Polygon#getBoundingRectangle generates a new Rectangle every time it's called. |
@badlogic Sure, I can remove the other methods if they're not considered necessary. |
If you keep getCenter, avoid creating new objects each time. Your Polyline#getCenter creates a Polygon and number of vertices instances of Vector2, and a Vector3. |
Also, Polygon#getBoundingRectangle only creates a new Rectangle if |
Sure, will do. |
I agree that a common However I'm not even sure whether |
implementing Shape
Why not a method like getVertices() to make it easier to generate a Mesh or a Box2D shape or something just given a Shape? |
The following changes were made in commit 7fd147c :
|
@JesseTG That would not make much sense, since you'd probably want to have a real BoxShape (or at least use Furthermore: Which vertices would |
Like @nooone, I'm on the fence as to whether getCenter actually adds any value. |
Since it's a 2D shape, I would prefer naming it |
@xoppa Does this mean we can hope for a future Shape3D implementation? |
Sure, we can always hope. |
@badlogic Can you make a decision here? I personally don't really see any value with the getCenter() method but you said it was OK, so you get to merge if you are OK with it as is. |
Let's kill getCenter() (sorry, was on honeymoon) and make it a tagging interface called Shape2D. |
getCenter() killed. |
Might as well kill the |
I think it should be good to go now. |
Sorry, just go back, merged, thanks! |
13 commits for a tagging interface, lol |
@nooone Yeah, stuff happens |
This would have been a good candidate for squashing commits. |
Unification of gdx.math shape classes under the Shape interface