GetShapes() nodisard when passing the count as capacity

[jhasse]

First of all thanks again for the library! I've ported our game jam project to it and it worked perfectly, it's much better than the old Box2D v2 C++ API.

One thing I've noticed is that when using the following pattern:

const auto count = body.GetShapeCount();
auto shapes = std::make_unique<b2ShapeId[]>(count);
body.GetShapes(shapes.get(), count);

I get Ignoring return value of function declared with 'nodiscard' attribute since GetShapes is [[nodiscard]]. But I guess that it might be quite common to just reserve the whole array up front like I did? Or maybe there should be a GetAllShapes method?

[HolyBlackCat]

I'm a bit hesitant to remove [[nodiscard]] from those. All the samples still use this return value, as if it can be less than the reported capacity.

I've looked at the source code, at least in some of the functions, the reported capacity seems to match the size exactly. Perhaps Erin is trying to be future-proof with this (in case they stop being equal later).

I recommend doing count = body.GetShapes(shapes.get(), count);.

[jhasse]

I guess the idea is to have the possibility to use a stack array, i.e.

b2ShapeId shapes[64];
auto count = body.GetShapes(shapes.get(), 64);

for performance. But I use this method for debugging only, so performance isn't important.

I recommend doing count = body.GetShapes(shapes.get(), count);.

Hm ... okay. I would think that if there isn't a bug in GetShapeCount() that this will never change the value of count. If it does I would want to know that there's something seriously wrong.

[HolyBlackCat]

For the record, here's one example of the samples not ignoring the return value despite knowing the capacity: https://github.com/erincatto/box2d/blob/c05c48738fbe5c27625e36c5f0cfbdaddfc8359a/samples/sample_events.cpp#L790

My understanding is that Erin leaves the possibility for GetCapacity() to be changed to return an estimate that's larger than the actual count. But ¯\_(ツ)_/¯, you could ask him what the intent is.

[jhasse]

In that example he passes a capacity, not the count that was received before.

Here's an official example where the return value is ignored: https://github.com/erincatto/box2d/blob/c05c48738fbe5c27625e36c5f0cfbdaddfc8359a/samples/sample_events.cpp#L1147

[HolyBlackCat]

Ah, I didn't realize some functions are called GetCapacity() and some GetCount(). I need to think what to do about it.