This work is Related to #3694 . The diff here will be smaller once #3694 is merged. Otherwise, this PR will be refactored into an independent patch.
(I was too lazy to break this branch away from the feature branch, is what I'm trying to say).
This is to fix some of the bugs I've noticed in the multibody graph traversal part of OpenSim. Notably:
It doesn't currently take the frame socket topology of Model into account, which leads to issues once you start doing things like chaining PhysicalOffsetFrames together or using StationDefinedFrames (which, themselves, depend on Stations, defined in PhysicalOffsetFrames, etc.)
Doesn't account for other socket topologies in the model, which is why some components have to manually call setNextComponentInSystem (when the socket system of OpenSim should, in principle, be enough to figure this out)
Segfaults under certain conditions, such as users creating unusual joint cycles in their models (e.g. https://github.com/ComputationalBiomechanicsLab/opensim-creator/issues/773). Those types of cycles are easy enough to create via a UI. They shouldn't result in a segfault (at worst, an exception is fine).
Brief summary of changes
Refactor/tidy up Model::extendConnectToModel (so I/we can see what it's trying to do)
Remove setNextSubcomponentInSystem calls that are peppered throughout Model::extendConnectToModel
Centralize almost all setNextSubcomponentInSystem calls to a single for (auto& component : TopologicallySortedComponents(model)) {} expression
Testing I've completed
Kicking it until it stops screaming, because it's a very central algorithm that's unlikely to squeeze past the test suite until every edge case is covered
WIP
This is to fix some of the bugs I've noticed in the multibody graph traversal part of OpenSim. Notably:
Model
into account, which leads to issues once you start doing things like chainingPhysicalOffsetFrame
s together or usingStationDefinedFrame
s (which, themselves, depend onStation
s, defined inPhysicalOffsetFrame
s, etc.)setNextComponentInSystem
(when the socket system of OpenSim should, in principle, be enough to figure this out)Brief summary of changes
Model::extendConnectToModel
(so I/we can see what it's trying to do)setNextSubcomponentInSystem
calls that are peppered throughoutModel::extendConnectToModel
setNextSubcomponentInSystem
calls to a singlefor (auto& component : TopologicallySortedComponents(model)) {}
expressionTesting I've completed
Looking for feedback on...
CHANGELOG.md (choose one)
This change is![Reviewable](https://reviewable.io/review_button.svg)