Should caches be compared in the equality operator for the world?

[opera-aberglund]

This is how the equality operator for the AabbTreeWorld is implemented:

bool operator==(const AabbTreeWorld& lhs, const AabbTreeWorld& rhs) noexcept
{
    return // newline!
        // skip m_tree, should just be a cache anyway
        (lhs.m_bodyBuffer == rhs.m_bodyBuffer) && // newline!
        (lhs.m_shapeBuffer == rhs.m_shapeBuffer) && // newline!
        (lhs.m_jointBuffer == rhs.m_jointBuffer) && // newline!
        (lhs.m_contactBuffer == rhs.m_contactBuffer) && // newline!
        (lhs.m_manifoldBuffer == rhs.m_manifoldBuffer) && // newline!
        (lhs.m_bodyContacts == rhs.m_bodyContacts) && // newline!
        (lhs.m_bodyJoints == rhs.m_bodyJoints) && // newline!
        (lhs.m_bodyProxies == rhs.m_bodyProxies) && // newline!
        (lhs.m_proxiesForContacts == rhs.m_proxiesForContacts) && // newline!
        (lhs.m_fixturesForProxies == rhs.m_fixturesForProxies) && // newline!
        (lhs.m_bodiesForSync == rhs.m_bodiesForSync) && // newline!
        (lhs.m_bodies == rhs.m_bodies) && // newline!
        (lhs.m_joints == rhs.m_joints) && // newline!
        (lhs.m_contacts == rhs.m_contacts) && // newline!
        (lhs.m_islanded == rhs.m_islanded) && // newline!
        // skip m_listeners, they're inconsequential & not very comparable anyway
        (lhs.m_flags == rhs.m_flags) && // newline!
        (lhs.m_inv_dt0 == rhs.m_inv_dt0) && // newline!
        (lhs.m_vertexRadius == rhs.m_vertexRadius);
}

Why are m_bodyContacts, m_bodyJoints, m_bodyProxies, m_proxiesForContacts, m_fixturesForProxies, m_bodiesForSync compared here, since they are only caches? My reasoning is that two worlds should still be considered equal despite having different caches, as long as all the bodies and shapes and so on are equal.

I'm wondering the same for Islanded, is this also some type of cache? Does two worlds with different values in Islanded behave differently? If so, I can't find a method to get these values so I can't copy them to another world.

[louis-langholtz]

Hmm... this requires some thinking. At least for me.

Caches shouldn't be part of equality, no. But, are the fields you're specifying exactly caches? Seems like they are, but I'm reluctant to agree on this without thinking carefully about it.

It's occurred to me before that m_bodies, m_contacts, m_joints for instance are cache-like. At least in the sense that they cache valid IDs (indexes) of their respective buffers - m_bodyBuffer, m_contactBuffer, m_jointBuffer. They also provide a user-specifiable ordering for processing however which isn't cache-like in my thinking of this and does affect processing results. This seems more subtle to me to realize and I'm concerned such subtly may be part of some of those other members too.

Incidentally, this relates to why there isn't a GetShapes accessor function. Because there isn't an m_shapes member. And that's because there wasn't a need for ordering for shapes beyond what's already in m_shapeBuffer. If I add a m_shapes member, that adds additional memory footprint to AabbTreeWorld which is unappealing.

Another way I think about dealing with the lack of a GetShapes function, is whether it'd be preferable to drop m_bodies, m_contacts, m_joints as well. This would make it harder for users to understand processing order related behavior I think but result in more cache friendly processing by always going in order of elements in the buffers then. It's not clear that processing order is something users usually care about anyway - just getting results which seem visually reasonable. So, dropping these would improve memory access patterns and reduce the memory footprint of AabbTreeWorld more.

The downside of course as you realize is not having the GetShapes like functions of GetBodies(world), GetJoints(world), etc.

OTOH, there are the Get*Range functions. Paired with a signal of whether a value between 0 and range indexes a valid entity, this could be used instead. IsDestroyed provides such a signal as a boolean result. Get{Body,Contact,Joint} functions provide a weaker form of that as default constructed results. I've considered the latter using IsDestroyed and then like throwing an exception if the index was to a destroyed entity, but calling IsDestroyed has some performance penalty for every access. So long as the user doesn't call Destroy however, 0 through Get*Range are valid and more efficient than going through indices via a GetBodies like container.

I'll continue thinking about the cache aspect of this and update this comment as time/decisions occurs.

[opera-aberglund]

Thanks for being so open about your reasoning, it's very interesting to follow. I wouldn't say this in itself is too much of an issue for me personally right now, but since I currently use this operator as a reference for the correctness of the serialization, it will eventually matter a lot before I can call it "complete".

Issue #555 is much more of a staller right now, because we need to use contacts in our game.