Extend WorldBody methods

[opera-aberglund]

Current behavior:

I think the methods defined in the WorldBody, WorldJoint, WorldShape ... and so on, are the best ones to use when updating the state (unless you are modifying multiple values at once and care a lot about performance). A couple examples:

void SetAcceleration(World& world, BodyID id, const LinearAcceleration2& linear,
                     AngularAcceleration angular)
{
    auto body = GetBody(world, id);
    SetAcceleration(body, linear, angular);
    SetBody(world, id, body);
}

void SetFriction(World& world, ShapeID id, NonNegative<Real> value)
{
    auto object = GetShape(world, id);
    SetFriction(object, value);
    SetShape(world, id, object);
}

Partly because they turn three lines of code into one, but mostly because it's so easy to otherwise forget to call SetBody(world, id, body) or SetShape(world, id, object) or whatever when you're done modifying the object. I believe it's especially easy for programmers new to C++ to forget that GetBody for example returns the body by value, and not by reference.

These "helper" methods does however not cover all the different setters of the objects which makes it a bit inconsistent.

Desired improvement:

All Set-methods to bodies, contacts, joints, shapes and what not, should have a corresponding "world" method equivalent that provides the functionality to modify the value directly in the World state.

[louis-langholtz]

So, for instance, per this, for Body instances there'd be new functions then with a signatures like the following as convenience functions to the named underlying settings:

void SetInvMassData(World& world, BodyID id, NonNegative<InvMass> invMass, NonNegative<InvRotInertia> invRotI);
void SetUnderActiveTime(World& world, BodyID id, Time value);

[louis-langholtz]

This issue brings up questions for me:

1. Is it a mistake to have these world-oriented "convenience" interfaces to begin with?
2. How can the maintenance overhead of keeping these synced be reduced? It's not hard for me to come up with ways to reduce it, but none are clearly winners for me. Let's say Body gets a new "setter", it'd be nice to not have to manually add a new world interface to match it, that in turn also begs for new unit test code to be added for the new interfaces to avoid losing test coverage. Internally the implementation could be done using a macro or functor taking interface. Doing the latter externally though then exposes the type to using code such that a convenience to modifying Body then effectively exports Body.hpp to the calling code which WorldBody,hpp currently does not.
3. Does furthering this "convenience" pattern not also begs for similar interfaces between say Body and BodyConf?
4. Is it already clear enough the difference between *Conf types and their respective component types? The former types are intended to be more LiteralType oriented, more constexpr friendly, cache-less, and free of invariants between member variables. The latter types are intended to be the opposite basically: not entirely constexpr, potentially having caches for speed optimizations, potentially having invariants between member variables, and enforcing those invariants.
5. Should component types like Body also support a builder pattern like the *Conf types tend to? Then the typical three line pattern could become one liners like SetBody(world, id, GetBody(world, id).Enabled(true)). This has the downside of exposing the component type - i.e. Body.hpp then gets pulled in to user's code - and also making users' code lines longer. An upside though may be potentially no longer wanting the current 3-line to 1-line conveniences as much then and not dealing with as much code maintenance.

[opera-aberglund]

1. Is it a mistake to have these world-oriented "convenience" interfaces to begin with?

When I first came across these, I actually thought it might be a mistake to have any methods other than these, because these are often better to use to make sure that the world state always remains synchronized to the modifications you make. However, I realized such a change might require exposing the specific setters of the components to the world implementation.

4. Is it already clear enough the difference between *Conf types and their respective component types? The former types are intended to be more LiteralType oriented, more constexpr friendly, cache-less, and free of invariants between member variables. The latter types are intended to be the opposite basically: not entirely constexpr, potentially having caches for speed optimizations, potentially having invariants between member variables, and enforcing those invariants.

I'm not sure if the difference is clear enough actually. Is it correct that the specific shapes and joints only have Conf types? Like the ChainShapeConf for example, is its corresponding component type just the Shape or is there a ChainShape somewhere as well?

5. Should component types like Body also support a builder pattern like the *Conf types tend to? Then the typical three line pattern could become one liners like SetBody(world, id, GetBody(world, id).Enabled(true)). This has the downside of exposing the component type - i.e. Body.hpp then gets pulled in to user's code - and also making users' code lines longer. An upside though may be potentially no longer wanting the current 3-line to 1-line conveniences as much then and not dealing with as much code maintenance.

I actually like this solution very much! It would simplify things a lot if the Conf types could just be dropped entirely in favor of having the component types implement the builder pattern directly themselves. I guess the reason why Box2D has these structs to begin with is because their objects don't have a builder pattern like PlayRho does, so those are used instead. When creating a new body it would be nice if it could look something like this instead:

CreateBody(world).UseType(BodyType::Dynamic).Use(shapes);

But then the CreateBody would have to return a Body instead of a BodyID, so there has to be some other way of getting the new ID back. Perhaps storing it directly as an immutable value in the body so you can just do body.GetID(). I'm just brainstorming ideas here though.

Removing them would also reduce maintenance because preferably the Conf types should always be enough to construct a perfect copy of an object. This was actually what I did first when serializing and deserializing the objects: I looked at the values in the Conf type and falsely concluded that these should be the ones enough to pass through. It was only after looking at the == operator that I realized there were some values that were missing in it (thus #549).

If going for this approach, I think I would just drop the convenience methods completely as well. Another upside of this is when you press the . in your IDE, the massive list of methods you see would shrink a lot, when now there can sometimes be like 8 different variants of the same function.

[louis-langholtz]

I appreciate the feedback. It's helpful for me.

Is it correct that the specific shapes and joints only have Conf types?

Currently, yes. Specific shape types and joint types are all named with a pattern of *ShapeConf or *JointConf respectively.

Incidentally, I have been considering dropping the Conf part of these types' names. Perhaps that's better put off to like a 3.0 release though. Would you prefer these types' names not ending with Conf?

it would be nice if it could look something like this instead CreateBody(world).UseType(BodyType::Dynamic).Use(shapes)

With respect to a coding pattern like that, that's not too different syntactically from the SetBody(world, id, GetBody(world, id).Enabled(true)) example I'd given.

But, I think the coding pattern you mention could run against the separation of spaces that I want. This is where getting the body from the world is like asking the kernel for a copy of kernel state, and changing the body is a copy back into the kernel. The kernel in this case conceptually being the World.

A setup I'm striving towards in this regard is being able to have worlds implementable on speciality hardware like GPUs without changing the user's interface. I think this also simplifies exporting the engine to other languages without having to reimplement it all in the other language.

I can imagine the separation could be maintained by having that kind of CreateBody return a new builder type that stored the BodyID along with BodyConf like state. Then on its destruction, or on like a Sync call, it'd set the state back into the world using the ID it stored.

[opera-aberglund]

Incidentally, I have been considering dropping the Conf part of these types' names. Perhaps that's better put off to like a 3.0 release though. Would you prefer these types' names not ending with Conf?

I would definitely prefer that!

I can imagine the separation could be maintained by having that kind of CreateBody return a new builder type that stored the BodyID along with BodyConf like state. Then on its destruction, or on like a Sync call, it'd set the state back into the world using the ID it stored.

That sounds like a kinda cool solution, as long as it doesn't become too complex for the average user to wrap their head around it. I was thinking another way to do it is to just call CreateBody with a Body, which I saw is already overloaded, and if the Body also implements a builder pattern like BodyConf, you can just keep it like:

CreateBody(world, Body{}.UseType(BodyType::Dynamic).Use(shapes));