louis-langholtz/PlayRho

Incomplete equality operator for Body

opera-aberglund opened this issue · 2 comments

I might be wrong here, but shouldn't this also compare the m_flags of the bodies?

bool operator==(const Body& lhs, const Body& rhs)
{
    return GetTransformation(lhs) == GetTransformation(rhs) && //
           GetSweep(lhs) == GetSweep(rhs) && //
           GetType(lhs) == GetType(rhs) && //
           GetVelocity(lhs) == GetVelocity(rhs) && //
           GetAcceleration(lhs) == GetAcceleration(rhs) && //
           GetInvMass(lhs) == GetInvMass(rhs) && //
           GetInvRotInertia(lhs) == GetInvRotInertia(rhs) && //
           GetLinearDamping(lhs) == GetLinearDamping(rhs) && //
           GetAngularDamping(lhs) == GetAngularDamping(rhs) && //
           GetUnderActiveTime(lhs) == GetUnderActiveTime(rhs) && //
           GetShapes(lhs) == GetShapes(rhs);
}

I think you're correct, yes. Marked this as bug in the library. I'll address this shortly.

Looks like GetType - which is used in determining equality (as you show) - is a result of some of the m_flag state. But, not all of the flags state. So that'd be short on checking fully for equality.

I don't want to give operator== special access to Body. So I'm adding a GetFlags() member function instead and will use that instead of GetType() in checking equality.

On a downside, this exposes the Flag enum and FlagsType alias more. Which I'd wanted to avoid.

On an upside, this would allow some of the accessor member functions which only checked m_flags to be removed in favor of the non-member functions doing their logic instead by using GetFlags().

Marking this as closed and addressed now per the commits in the recent PR.