Setting Body.Enabled = false inside of OnCollision causes crash.

[poohshoes]

I'm running a 4 year old version of Farseer Physics so if this is not an issue with this version please just mark as resolved.

I'm getting a crash in Contact.Update if the FixtureA OnCollision handler has the code Body.Enabled = false then the FixtureB will be null causing a crash before the FixtureB handlers can be called.

Currently I'm going to just do if(FixtureB != null so it won't crash as I can't see any side effects but OnCollision would also have to check if you had more than 1 collision handler attached.

[nkast]

You can't set Body.Enabled while the simulation is running. (box2D code here) You can ignore the collision by returning false from OnCollision. You can remove or disable the body later, after World.Step() is finished.

[poohshoes]

Right but it's not an unreasonable thing to want to do in response to a collision. So if you need to set a bool and then disable it after World.Step is finished why not have farseer take care of it automatically?

And wouldn't it be easier to have that code in Farseer than for everyone who uses it to have to write around it?

Also if you aren't supposed to set Body.Enabled while the simulation is running perhaps it should give you a warning? The whole point of a library is to not have to dig around in the code and figure out what the issue is.

[Genbox]

@nkast The engine is different from Box2D in that you can actually set Body.Enabled in the OnCollision handler. I'm pretty sure I fixed this bug years ago and made a test for this particular scenario. See https://github.com/VelcroPhysics/VelcroPhysics/blob/master/Samples/Testbed/Tests/LockTest.cs

@poohshoes The fix you made is pretty much what solves it. There are multiple gotchas in event handling in C#, which is why I'm probably moving towards #8 with Velcro rather than having the engine internals change in the middle of a step. As for having the engine remember Body.Enabled changes during a step, that would considerably slow down the engine and take up a lot of memory - the Enabled property is just one out of many things that can change.

[nkast]

It's hard to find the exact fix, maybe I will try to find it tomorrow, however there are a couple of issues that I think that are not addressed.

The other fixture does not get notified about the contact. BeginContact too (or maybe BeginContact gets an empty Contact). Small issue really but might be helpful, Ex. a bullet (A) disables itself and the ship (B) gets ++Hits.

A body (B) receives OnSeperation before OnCollusion. When a body is disabled it will remove/destroy all of it's contacts. This behavior puts your game object on an invalid state.

The index/Iteration of contacts is not corrected. So when a body or two is disabled, Collide()/SolveToi(), etc might skip one or two contacts. That's somehow similar to #41.

[nkast]

Also LockTest works only when OnSeperation is fired from the second last event.

if the user disable a body from within the first event, https://github.com/VelcroPhysics/VelcroPhysics/blob/e25af123fdb664b79e65d1d11224a2abc978a5e6/VelcroPhysics/Collision/ContactSystem/Contact.cs#L279

then the next line will throw a null exception because FixtureB is now null. https://github.com/VelcroPhysics/VelcroPhysics/blob/e25af123fdb664b79e65d1d11224a2abc978a5e6/VelcroPhysics/Collision/ContactSystem/Contact.cs#L280

That's also true on Farseer 3.5 https://github.com/VelcroPhysics/VelcroPhysics/blob/8e5e8013fc6a1baeea8c34e5dbd20029b0f05f84/SourceFiles/Dynamics/Contacts/Contact.cs#L302-L312

[Genbox]

You are completely right, which is why it is a bad idea to change the state of the engine in the middle of a step. it is possible to do because it works for most games.

[nkast]

going back to the idea of automatic disabling, I was thinking of a World.WaitHandler, so you can write something like fixtureB.Body.World.WaitHandle.ContinueWith( (w)=>fixtureB.Enabled = false ) or provide Async methods like EnableAsync() and DisableAsync().

I am not sure if I can implement those without generating garbage. However it would be nice for quick prototyping and rare events.