Help Problems deleting obi colliders with pin constraints

[btduser]

Hello, I'm trying to do something seemingly trivial - to cleanly delete an object with an ObiCollider and not have my pin constraints blow up - but apparently it's complicated.

I'm creating these pins via scripting, following examples from the documentation. The pinning works fine until I destroy a collider in my scene. When that happens, my console starts getting spammed with the following:

Code:

System.IndexOutOfRangeException: Index 15 is out of range of '15' Length.

This Exception was thrown from a job compiled with Burst, which has limited exception support.

[redacted rest due to forum anti-spam]

After a lot of excruciatingly painful debugging, I finally have an idea of why this is happening, but no idea how to proceed.

From what I have gathered, the issue is due to the fact that when I destroy (or disable) a collider, this changes the number of items in `ObiColliderWorld.instance.colliderHandles`, but the collider handle indexes in the pin constraints don't ever get updated. This also happens if I disable the object. Presumably because in either case, `ObiColliderBase.RemoveCollider` is invoked, which modifies the collider handle list.

This either spams burst errors or causes the rope to jump to completely unrelated obi colliders. Even just disabling and re-enabling an obi collider will steal ropes from other objects - e.g. if I have 10 colliders in my scene, with a pin on collider A at index 9, disabling collider B at index 5 rearranges the list so that A is now at index 8 and index 9 is invalid, which will spam errors until I re-enable B, which causes B to get assigned index 9 again, and now the pin is pointing to B instead of A.

I have no idea how to proceed. I've tried deleting the collider at different steps of the process (using the actor callbacks) in case it was timing-related, but the issue persists. Explicitly calling `rope.SetConstraintsDirty(Oni.ConstraintType.Pin);` after destroying my object doesn't seem to fix anything either.

I don't know what I'm doing wrong - it seems like the issue is a fundamental one, that removing any collider in the scene has the potential to break completely unrelated pin constraints. The only solution I can think of is to modify my pins any time any obi collider is destroyed, but I don't see how that's feasible, since there's no way to make pins immediately aware of the change.

I'm using obi 6.5.4 with burst backend (burst 1.8.14), and the problem persists with oni backend as well (ropes just jump to wrong colliders instead of throwing exceptions). I'm basically at my wit's end here - any help is appreciated.

[josemendez]

From what I have gathered, the issue is due to the fact that when I destroy (or disable) a collider, this changes the number of items in `ObiColliderWorld.instance.colliderHandles`, but the collider handle indexes in the pin constraints don't ever get updated. This also happens if I disable the object. Presumably because in either case, `ObiColliderBase.RemoveCollider` is invoked, which modifies the collider handle list.

This either spams burst errors or causes the rope to jump to completely unrelated obi colliders. Even just disabling and re-enabling an obi collider will steal ropes from other objects - e.g. if I have 10 colliders in my scene, with a pin on collider A at index 9, disabling collider B at index 5 rearranges the list so that A is now at index 8 and index 9 is invalid, which will spam errors until I re-enable B, which causes B to get assigned index 9 again, and now the pin is pointing to B instead of A.

Hi!

Think about what happens when you destroy an element in a list: either all elements that appear after the index of the destroyed elements move one space towards the start of the list, or alternatively you can move the last element in the list to the "gap" left by the destroyed element (a swapback operation) if relative ordering of objects is not important. Either option will change the indices of objects in the list.

I don't know what I'm doing wrong - it seems like the issue is a fundamental one, that removing any collider in the scene has the potential to break completely unrelated pin constraints.

Correct. Burst can't deal with object references as it can only work with blittable types. To keep a reference of sorts to an object, indices must be used, but that means that the indices need to be updated whenever objects get moved around in memory.

The only solution I can think of is to modify my pins any time any obi collider is destroyed, but I don't see how that's feasible, since there's no way to make pins immediately aware of the change.

This is the correct solution and it is what Obi does internally. Same applies to all constraints, not just pin constraints: whenever the index of a particle or collider in the internal solver arrays change, all constraints affecting them must be re-created or updated. Remember we are dealing with indices, not pointers/references, so they are not automagically updated.

This is why the ObiColliderHandle class exists: it stores an object reference to the actual Collider, and its index in the solver arrays. This index is updated automatically by the collision system.

When you create a pin constraint using batch.AddConstraint(), the handle to this collider is stored in the batch. Any subsequent calls to
rope.SetConstraintsDirty(Oni.ConstraintType.Pin); will force the constraints to use whatever collider index is currently stored in the handle - as long as the constraint has been added to the actor, and not the solver. If you add constraints directly to the solver you're on your own when it comes to updating them anytime particles/colliders change.

See how ObiParticleAttachment does it: it subscribes itself to solver.Actor_OnPrepareStep, where it calls UpdateAttachment() which in turn calls SetConstraintsDirty if the index of the collider has changed.

kind regards,

[btduser]

Hi, thanks for the quick reply. I guess I'm still confused as to how I should best resolve the issue. Referring to what you said:

This is the correct solution and it is what Obi does internally. Same applies to all constraints, not just pin constraints: whenever the index of a particle or collider in the internal solver arrays change, all constraints affecting them must be re-created or updated. Remember we are dealing with indices, not pointers/references, so they are not automagically updated.

This is why the ObiColliderHandle class exists: it stores an object reference to the actual Collider, and its index in the solver arrays. This index is updated automatically by the collision system.

Here is an example of how I'm creating a pin constraint:

Code:

pinConstraints = rope.GetConstraintsByType(Oni.ConstraintType.Pin) as ObiPinConstraintsData;
pinBatch = new();
targetObiCollider = gameObject.AddComponent<ObiCollider>();
pinBatch.AddConstraint(
    rope.elements[0].particle1,
    targetObiCollider,
    Vector3.zero,
    Quaternion.identity, 0, 10000, 1);
pinBatch.activeConstraintCount = 1;
pinConstraints.AddBatch(pinBatch);
rope.SetConstraintsDirty(Oni.ConstraintType.Pin);

As you can see, I am not directly referencing the collider handle - just passing `targetObiCollider` to the API.

When you create a pin constraint using batch.AddConstraint(), the handle to this collider is stored in the batch. Any subsequent calls to

rope.SetConstraintsDirty(Oni.ConstraintType.Pin); will force the constraints to use whatever collider index is currently stored in the handle - as long as the constraint has been added to the actor, and not the solver. 

As far as I can tell, I'm not adding the constraint to the solver, since I performed the `GetConstraintsByType` call on the rope (actor) and not its solver. Or is my above code wrong?

Regardless, I noticed that `ObiPinConstraintsBatch.AddConstraint` is doing two things: 1) it tracks the collider handle in pinBodies 2) it caches the current handle index in `colliderIndices`. I had originally assumed that `rope.SetConstraintsDirty(Oni.ConstraintType.Pin);` would cause the collision system to update the index value in `colliderIndices`, because it is already keeping track of the handle, but this doesn't actually seem to be happening.

If the only possible solution is to duplicate the logic in `ObiParticleAttachment.UpdateAttachment`, that's fine. It seems pretty heavy-handed to require every single pin I create to then have to continuously poll for changes instead of having the collision system resolve this internally, which is what I was hoping for. I'll probably need to write an entire helper class for this though.

[josemendez]

As you can see, I am not directly referencing the collider handle - just passing `targetObiCollider` to the API.

That's ok, ObiCollider already contains a reference to its handle. I just mentioned that the handle to that collider is stored by the batch when you call AddConstraint, not that you had to explicitly pass a handle yourself.

Regardless, I noticed that `ObiPinConstraintsBatch.AddConstraint` is doing two things: 1) it tracks the collider handle in pinBodies 2) it caches the current handle index in `colliderIndices`.

Correct, this is what I was talking about in my previous post:

When you create a pin constraint using batch.AddConstraint(), the handle to this collider is stored in the batch. Any subsequent calls to
rope.SetConstraintsDirty(Oni.ConstraintType.Pin); will force the constraints to use whatever collider index is currently stored in the handle

I had originally assumed that `rope.SetConstraintsDirty(Oni.ConstraintType.Pin);` would cause the collision system to update the index value in `colliderIndices`, because it is already keeping track of the handle, but this doesn't actually seem to be happening.

That's exactly what SetConstraintsDirty does: it triggers an update of the collider index (and other data) for all pin constraints.

SetConstraintsDirty activates a "dirty" flag for that specific type of constraint. Next time the solver is updated, the constraint's Merge() method is called for any constraint types that were flagged as dirty. This method takes whatever constraint data is stored in all actors, reorders/batches it together in as few batches as possible for efficient parallelization, and copies their data over to the solver. In case of PinConstraints, this is implemented by ObiPinConstraintsBatch.Merge() which copies whatever collider indices are currently stored in the handles to the solver's constraint arrays.

Note that since SetConstraintsDirty just sets them "dirty" but doesn't actually update them, the change is not immediate and only takes place right before the next physics update.

Also note this means you shouldn't destroy colliders/constraints/particles during the physics update, since this would basically pull the rug from under the solver's feet. Maybe this is what's happening in your case?

A quick summary of what happens under the hood:
- A collider is deleted -> collision system swaps it with the last active collider and reduces the number of active colliders by one -> the collider index for both collider handles is updated.
- Any call to SetConstraintsDirty() tells the solver to update affected constraints at the start of next update.
- As a response to SetConstraintsDirty, constraints.Merge is called which takes the collider index in the handle and copies it over to the solver's constraint arrays.

If the only possible solution is to duplicate the logic in `ObiParticleAttachment.UpdateAttachment`, that's fine. It seems pretty heavy-handed to require every single pin I create to then have to continuously poll for changes instead of having the collision system resolve this internally, which is what I was hoping for.

Attachments only pool for changes to avoid calling rope.SetConstraintsDirty every frame regardless of whether there's been a change - something you could also do. Note there isn't any need to cache the current index and check whether it has changed or not, we do this just to avoid some extra overhead since use cases with thousand of attachments (only case where the cost of polling would show up in profiling) aren't common at all. Either way, the collision system does resolve this internally.

let me know if you need further help.

[btduser]

I think I finally figured out the issue; the problem was that I have multiple ropes in my scene, where each one has its own solver, and each solver is registered to one obi fixed updater. When I was marking the pin constraints as dirty, it was only affecting one of the ropes but the rest remained ignorant of the index change. My solution is to call this instead:

Code:

public void NotifyColliderDeleted() {
    int dirtyMask = 1 << (int)Oni.ConstraintType.Pin;
    for (int i = 0; i < fixedUpdater.solvers.Count; i++) {
        fixedUpdater.solvers[i].dirtyConstraints |= dirtyMask;
    }
}

With this, I don't need each pin to actively poll for changes.

So in summary, if an obi collider is destroyed anywhere in the scene, all solvers need to be made aware by having their constraints marked as dirty. A simple issue, but really hard to find since the index errors I was getting gave me zero context.

[josemendez]

I think I finally figured out the issue; the problem was that I have multiple ropes in my scene, where each one has its own solver, and each solver is registered to one obi fixed updater.

Not sure if by “each solver is registered to one updater” you mean each solver has its own updater, or that there’s just one updater. Note that having each solver be updated by their own updater means they will all be updated sequentially, making no use of multithreading. This usually has a negative impact on performance.

The typical setup would be to have just one updater in your scene, there’s little point in having multiple updaters, unless you want specific actors to be updated at different moments during the frame. See: https://obi.virtualmethodstudio.com/manu...aters.html

[btduser]

Not sure if by “each solver is registered to one updater” you mean each solver has its own updater, or that there’s just one updater. Note that having each solver be updated by their own updater means they will all be updated sequentially, making no use of multithreading. This usually has a negative impact on performance.

The typical setup would be to have just one updater in your scene, there’s little point in having multiple updaters, unless you want specific actors to be updated at different moments during the frame. See: https://obi.virtualmethodstudio.com/manu...aters.html

I mean there is one updater in the scene, with multiple solvers. I needed to dirty every solver, however, and not just the one logically involved with the ObiCollider I'm destroying.

[josemendez]

I mean there is one updater in the scene, with multiple solvers.

Ok! sounds good then.

I needed to dirty every solver, however, and not just the one logically involved with the ObiCollider I'm destroying.

Yes, since colliders are shared among all solvers. Otherwise we would have to keep multiple copies of the colliders in memory (one copy per solver) and complicate setup to determine which colliders are involved with which solvers.