Create intermediate segments in ConcavePolygonShape2D.

[madmiraal]

Fixes #19353, fixes #29320.

A ConcavePolygonShape2D is expected to be similar to a ConvexPolygonShape2D. The description for ConcavePolygonShape2D states:

The main difference between a ConvexPolygonShape2D and a ConcavePolygonShape2D is that a concave polygon assumes it is concave and uses a more complex method of collision detection, and a convex one forces itself to be convex in order to speed up collision detection.

This patch makes them similar by using the points in the PoolVector2Array associated with a ConcavePolygonShape2D to create a contiguous, closed-loop of segments instead of requiring and grouping them into pairs of discontiguous segments, which leaves unexpected holes between the pairs.

[bojidar-bg]

I tagged this PR as "breaks compat" since I'm not sure if existing point arrays would work fine with the new changes. I guess that zero-length segments could be filtered out so that well-formed old-format arrays of points continue to work optimally?

[madmiraal]

@bojidar-bg The "breaks compat" is valid; since the ConcavePolygonShape2D could have been used to create discontiguous collections of segments, which would now be linked together. However this would only apply to those using this unexpected behaviour as a feature.

On your second point, I did look at removing zero length segments, but the problem was the edge case of one point. With only one point, one zero length segment is created that loops back to itself. Deleting this segment deletes the only segment and hence the one point. This prevents the array size from being incremented from 0. Since the original only removed duplicated points not zero length segments, I chose to keep it simple and not add this.

[madmiraal]

Rebased following #36311.

[madmiraal]

Rebased following rename of VisualServer.

[madmiraal]

Rebased following #38738.

[Xrayez]

Would be really nice to have this merged now, trying to draw a ConcavePolygonShape2D via script with various success rates, see #37903.

[Xrayez]

Tested for simple scene with static and rigid bodies (both inside and outside the static collider).

concave-shape-segments.zip

[reduz]

My feeling with this APIs is that they should take a bit more pre-munched data, otherwise the servers need to generate the support structures, like BVH for concave, and the list of triangles/vertices/edges for the convex stuff instead of this high level stuff. I was thinking of breaking compat more towards this discretion in the 4.0 branch.. then leave the previous API more of as a helper if you want to pass simpler data

[Xrayez]

If most of the physics stuff is going to be rewritten in 4.1 or so, then yeah I don't see much point in breaking compat this way. But from what I see in this PR, this is mainly just a consistency fix for what a user can expect from both concave and convex polygon shapes in terms of input. To each problem, its own solution, and this PR does fix some.

One small step for 4.0, one giant leap for 4.1. 🙂

[akien-mga]

Is this still relevant for the current master branch? IIRC there were some improvements merge to make it easier to edit these segments from the editor.

CC @godotengine/physics

[madmiraal]

Yes, it's still relevant. It still fixes both #19353 and #29320, which are both still valid issues.

[KoBeWi]

I can confirm it fixes both issues (although the first one is rather vague) and breaks compat. Can't say much about the code.

This change makes ConcavePolygonShape2D basically the same as CollisionPolygon2D; at least interface-wise (minus the editor), but maybe the collision checks work differently internally, idk.

[reduz]

Not in favor myself of this change. It only limits the API and removes the ability to use multiple shapes (currently used by Sprite2D -> collision generation) as well as open shapes (say terrain) with this API. Instead, what we could do is rename this resource to MultiSegmentShape2D, which is more faithful to what it is.

[KoBeWi]

Is there any advantage of using it, over multiple polygons?

[lawnjelly]

Closing as we didn't go ahead with this approach.