Missing properties for BodyConf about playrho HOT 7 CLOSED

Not agreeing or disagreeing, just making some notes:

• Code in Body::Body I see that the sweep is initialized from the location and angle member variables of the given BodyConf. This wouldn't appear to, by-itself, preserve the state of the sweep's pos, localCenter, nor alpha0 member variables.
• Code in Body::Body initializes m_invMass but not to a specific inverse mass specifiable by the given BodyConf.

from playrho.

With Sweep and others that are their own strong type, I'm inclined to call the function just Use - so like .Use(sweep). At least that's more consistent with what I've been doing for unique/strong types. I imagine that'd be preferable to you too, no?

from playrho.

Not agreeing or disagreeing, just making some notes:

• Code in Body::Body I see that the sweep is initialized from the location and angle member variables of the given BodyConf. This wouldn't appear to, by-itself, preserve the state of the sweep's pos, localCenter, nor alpha0 member variables.
• Code in Body::Body initializes m_invMass but not to a specific inverse mass specifiable by the given BodyConf.

Yes, these were the things I noticed as well that could diverge after the body has been created and modified, why I had to set them manually after the BodyConf to preserve the state.

With Sweep and others that are their own strong type, I'm inclined to call the function just Use - so like .Use(sweep). At least that's more consistent with what I've been doing for unique/strong types. I imagine that'd be preferable to you too, no?

Works either way for me!

from playrho.

location and angle of BodyConf are two of the fields for the Body Sweep. Adding the entire Sweep data to BodyConf I'm considering as something that could be done by adding the missing state as additional member variables of BodyConf, or by subsuming location and angle into a Sweep member variable instead.

A downside I can think of for adding the additional state of Sweep via additional member variables is the additional builder functions then that need to be also added. An upside of course is this wouldn't require changes to any users' code who are relying on these interfaces already.

The master branch is not intended to be as stable though unlike release branches.

An upside to subsuming location and angle members of BodyConf with instead a Sweep variable would be gaining the use of another strong type instead of these two that are only strong with BOOST units support enabled. Another upside is in this reducing the number of builder function interfaces then to support its use.

from playrho.

I just merged changes which I believe resolve all of the missing properties issue. See the PR #552 for details. Turned out to be more work than I had expected.

PlayRho had lots of direct uses of the BodyConf location and angle member variables which had to be removed to fully support Sweep and that using code had to be updated to use functions instead to maintain the intended state settings.

Another impediment for me in this changes, was considering the semantics of the replaced/added state and ensuring sensible variance per the BodyType and other possible settings. I keep thinking I'd save myself headache by switching BodyConf to instead be like an alias to std::variant<StaticBodyConf, KinematicBodyConf, DynamicBodyConf>. Where, logically speaking, KinematicBodyConf is all the state of StaticBodyConf plus velocity oriented state, and DynamicBodyConf is all the state of KinematicBodyConf plus acceleration oriented state.

Please let me know if you think I may have missed any state.

from playrho.

Marking this as closed/resolved for now.

from playrho.

I'm testing this out now and I'm having a small issue with it. I removed the calls to UseLocation and UseAngle and replaced them with UseSweep. However, the resulting body doesn't have the same location, or "m_xf", as the one I serialized it from. It seems that the value of m_xf is set from the pos0 from the sweep, done from the GetTransformation here:

Transformation GetTransformation(const BodyConf& conf) noexcept
{
    return {conf.sweep.pos0.linear, UnitVec::Get(conf.sweep.pos0.angular)};
}

when the value of x_mf in the old body is the value of pos1 of the sweep. And I can't call UseLocation to fix this, because then I will loose the pos0 from the sweep.

I also had to set resetMassData to false, because otherwise I will also loose the pos0 from the sweep, but I guess I always have to avoid this function if I want an exact copy of a body?

from playrho.