No way to handle failed b2PolygonShape::Set

[briansemrau]

When b2PolygonShape::Set fails, whether it be because N<3, the polygon is degenerate, or otherwise, the function currently fails an assertion and calls SetAsBox. This makes it impossible to check if we've passed a bad polygon to the function, which is crucial in projects where polygons can be fully defined by the user.

Right now the only way around this is to copy the logic from within b2PolygonShape::Set to check the validity of our input before actually calling b2PolygonShape::Set.

I propose that instead of calling b2Assert and failing, b2PolygonShape::Set should simply return false (and continue to call SetAsBox as currently implemented - that part is good)

[indianakernick]

In the case where the polygon is defined by the programmer, an assert is desirable. With your proposed changes, that would require changing this

shape.Set(vertices, count);

to this

[[maybe_unused]] const bool valid = shape.Set(vertices, count);
b2Assert(valid);

to maintain the same behaviour. I think the current Set function should be kept as it as with a separate function for validation.

[briansemrau]

I'd generally argue against using asserts in functions with nontrivial failure cases. There's no reason to assume that every polygon is going to be made using predefined data. However, I can also see many reasons why it is done that way, so I agree that adding a function to pre-validate a set of points would be the best solution to this.

[erincatto]

I will add a function to check for validity.

[franklange]

Heyhey, just stumbled across this issue. Looking at the b2PolygonShape::Set code I'm kinda curious how this is going to work out because Setbasically runs 2 internal algorithms that can lead to function abort: unique-folding and this Gift Wrap thingy.

So the polygon validator would have to run those 2 as well and then you can call Set with your validated poly which runs them again? (because it doesn't/can't rely on only getting valid polys)