godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Replace the `CollisionPolygon2D` node with a polygon shape resource for `CollisionShape2D` #2960

Open jiqz opened 3 years ago

jiqz commented 3 years ago

Describe the project you are working on

I am creating a game with PhysicsBody2Ds

Describe the problem or limitation you are having in your project

I find it slightly unintuitve that CollisionPolygon2Ds and CollisionShape2Ds are different nodes when they could be combined.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

By removing CollisionPolygon2Ds and rather only using \Concave/Convex\Polygon2Ds as the Shape in CollisionShape2Ds (given that the easy-to-use coordinate plotter and points editor is transferred into it), those learning how to use CollisionObject2Ds would find it easier to grasp the concept that they need a CollisionShape2D, than how it currently stands, needing a CollisionShape2D or a CollisionPolygon2D.

This would also make the nodes more logical and neater, and the documentation clearer and more concise, no longer risking forgetting to mention CollisionPolygon2Ds as well as CollisionShape2Ds.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Remove the CollisionPolygon2D node, implement the CollisionPolygon2D's polygon editor in \Concave/Convex\Polygon2Ds so that they can be as easily editted, remove/correct mentions of CollisionPolygon2Ds in the documentation.

If this enhancement will not be used often, can it be worked around with a few lines of script?

This enhancement changes the nodes, not the functions of nodes, but you could still use CollisionPolygon2Ds as a separate node. However, you wouldn't get the benefits (straightforwardness and simplicity)

Is there a reason why this should be core and not an add-on in the asset library?

This change is partially intended at new users of Godot, most of whom would not install add-ons from the asset library when first starting out. It also is a change which makes the nodes more concise, and by implementing this in the core it makes Godot as a whole more concise and better.

Note: As a lot of nodes are being reworked for the 4.0 release, I believe that should be the version this change would be implemented in.

Calinou commented 3 years ago

In the interest of making high-quality proposals, please read the rules and fill in the issue template. This proposal will be reopened once this is done :slightly_smiling_face:

jiqz commented 3 years ago

In the interest of making high-quality proposals, please read the rules and fill in the issue template. This proposal will be reopened once this is done 🙂

Sorry about that, I've now modified my proposal to fit the template 😉.

Xrayez commented 3 years ago

I believe Goost already solves the underlying use case with PolyCollisionShape2D node:

image

Instead of using resources such as CircleShape2D, PolyCollisionShape2D uses basic polygon-based nodes to define the final collision shape.

Additionally, you can perform boolean operations between PolyNode2D-derived classes such as PolyCircle2D (which solves #200). You can still use CollisionPolygon2D and CollisionShape2D nodes along with PolyCollisionShape2D. Unlike CSG nodes in Godot, this kind of approach also allows you to define collision shapes for RigidBody2D nodes, and the base class PolyShape2D can be extended via script to make other stuff, like custom navigation nodes etc.

You can also apply custom textures for PolyNode2D-derived nodes, much like Polygon2D in Godot.

I'd still recommend using CollisionShape2D if you'd like to have proper collision response for circle-based shapes. A polygon built to represent a circle would be only an approximation of it, and depending on your use case, might not be a good solution.

You can download Goost editor to try this for yourself. Once you download the executables, extract the contents and run the editor with the example project provided below:

Example project goost_poly_collision_shape.zip


Update: I may have misunderstood the purpose of this proposal, but the solution above still solves the problem, but from the other point of view, this shows that having both CollisionPolygon2D and CollisionShape2D might not be necessarily a bad design. In any case, that's a nice alternative to consider if you heavily rely on polygons to define your shapes.

Having said that, in order to keep this proposal on topic, feel free to ask questions at https://github.com/goostengine/goost/discussions, thanks!

Xrayez commented 3 years ago

I forgot to mention that there are also potential problems with having to redesign this from the editor side. CollisionPolygon2D is recognized as a node that uses AbstractPolygon2DEditor internally, and AbstractPolygon2DEditor is also used for other stuff like editing navigation polygons (at least in Godot 3.x).

The fact that collision shapes are broken down into ConvexPolygonShape2D and ConcavePolygonShape2D makes it extra complex, because you have to take into account whether the polygon that you edit ends up being convex (by decomposition), again this is done internally. We would kind of need to merge ConvexPolygonShape2D and ConcavePolygonShape2D into PolygonShape2D resource. See also godotengine/godot#21394.

Due to this, that's probably why CollisionPolygon2D was created as a separate node, and ConcavePolygonShape2D is mostly useful via code as of now. But in theory, both convex and concave shapes could be edited, it's just that we'd need to ensure that convex one is really broken down into smaller convex shapes if what we edit with a mouse is actually a concave polygon, and if we edit concave shape, make sure that segments that represent the shape do not self-intersect. Concave shape is more like PolySegmentShape to be honest, as evident from CollisionPolygon2D's build mode property.

That's also a reason why I have another proposal for refactoring collision shape editor: #1157. But then more likely than not, we wouldn't have to do that if everything was in a single place to begin with.

popcar2 commented 1 year ago

If this is still planed for Godot 4, it would be great if this could happen sooner than later. This change could potentially break a lot of projects as more people start using v4 now that it's relatively stable.

Calinou commented 1 year ago

If this is still planed for Godot 4, it would be great if this could happen sooner than later. This change could potentially break a lot of projects as more people start using v4 now that it's relatively stable.

4.0 is in feature freeze, so any changes will have to wait for a future 4.x release. Compatibility handlers will have to created if this feature is to be implemented in a 4.x release, so that it's possible to upgrade a project without manual changes.

Shadowblitz16 commented 1 year ago

I believe Goost already solves the underlying use case with PolyCollisionShape2D node:

image

Instead of using resources such as CircleShape2D, PolyCollisionShape2D uses basic polygon-based nodes to define the final collision shape.

Additionally, you can perform boolean operations between PolyNode2D-derived classes such as PolyCircle2D (which solves #200). You can still use CollisionPolygon2D and CollisionShape2D nodes along with PolyCollisionShape2D. Unlike CSG nodes in Godot, this kind of approach also allows you to define collision shapes for RigidBody2D nodes, and the base class PolyShape2D can be extended via script to make other stuff, like custom navigation nodes etc.

You can also apply custom textures for PolyNode2D-derived nodes, much like Polygon2D in Godot.

I'd still recommend using CollisionShape2D if you'd like to have proper collision response for circle-based shapes. A polygon built to represent a circle would be only an approximation of it, and depending on your use case, might not be a good solution.

You can download Goost editor to try this for yourself. Once you download the executables, extract the contents and run the editor with the example project provided below:

Example project goost_poly_collision_shape.zip

Update: I may have misunderstood the purpose of this proposal, but the solution above still solves the problem, but from the other point of view, this shows that having both CollisionPolygon2D and CollisionShape2D might not be necessarily a bad design. In any case, that's a nice alternative to consider if you heavily rely on polygons to define your shapes.

Having said that, in order to keep this proposal on topic, feel free to ask questions at https://github.com/goostengine/goost/discussions, thanks!

why isn't this part of the engine? it seems like really useful features to have and it would be nice for 3d as well

vvvvvvitor commented 2 weeks ago

Just found myself needing this as shape_owner_get_owner only returns Shape2Ds