Problem with multiple character controllers

[ecosky]

Hi,

I seem to have discovered a problem with using multiple character controllers and multithreading. I'm not sure exactly what the problem is yet, but I was able to isolate the behavior in the character playground demo and force it to reliably throw an exception. In the screenshot below, the info.Contact is null. Looks like something is manipulating the list while it is being evaluated. I haven't figured it out yet but I thought I should post what I found in case there is something I am overlooking that is causing the problem, and if not maybe Norbo can take a look at it sometime.



The changes to the character playground were to create 40 additional characters and provide them random keyboard input for the E/S/D/F and A keys. It's obviously a contrived situation, but it is based on a real situation that occurred in the game I am currently working on.

I have attached the modified CharacterPlaygroundDemo.cs file, you should be able to just copy it over and run the sample to see the problem in short order.

Thanks again for Bepu, it's been a great system and I am sure this bug will be sorted out soon (if it is a bug and not some kind of usage error).

[ecosky]

I think the problem might EDIT: partially stem from this bit of code in CharacterController.cs,257 which is clearing the contacts within the context of BEPUphysics.UpdateableSystems.BeforeSolverUpdateableManager.MultithreadedUpdate. Disabling this stopped the exception from happening, but now of course I need to consider what the side effects are and how to correct it.

Code: Select all

                    
                    //Prevent any old contacts from hanging around and coming back with a negative depth.
                    foreach (var pair in Body.CollisionInformation.Pairs)
                        pair.ClearContacts();

Edit: Hit another exception before I realized there are several other spots clearing contacts in CharacterController.cs that needed to be disabled. The investigation continues..

[Norbo]

Any modification to the pairs in a nonsynchronized context can potentially confuse parallel readers. Calls to pair.ClearContacts are one such modification. There's another area which force-updates the pair as well.

These modifications can technically be removed, but it can have a negative effect on quality in some cases. The character might 'hitch' on things that aren't there anymore for one frame, for example.

I'll go ahead and throw in the necessary synchronization- thanks for finding this!

[Norbo]

Oof, I found an issue that will be particularly tedious to fix. This might take me a little longer than anticipated; in the interim, getting rid of the line in the constructor that sets IsUpdatedSequentially to false will eliminate the issues at the cost of performance.

[ecosky]

I had a feeling this one might get a little complicated.. looking forward to seeing what was needed to fix it.
Thanks again for all your help.

[Norbo]

It should now be fixed on the development fork.

It wasn't quite as bad as I thought it was going to be at first glance; I opted for a conditional sledgehammer approach and avoided a bunch of nasty microlocks all over the place.

Implementing the fix for the character led me to discover a theoretical solver deadlock bug introduced back in the rewrite of v0.15.0, too. It's astronomically rare and I've never seen it happen nor heard any reports of it, but it's fixed!

[ecosky]

I've been using this change for a while and it's been working great. Thanks again for that, and sorry I didn't respond again to the thread sooner (I thought I had).

I just wanted to add one more thing to this which is I had a bug related to my using the entity tag for something other than a reference to the character controller. This made the LockCharacterPairs not actually lock character pairs as intended. It might be worth having a (perhaps #if DEBUG) check in the CharacterController update that verifies the tag is pointing to CharacterController because it might save someone else a day of debugging somewhere I haven't yet figured out what I'm going to do in place of using the tag for my own object yet, but it shouldn't be too big of a deal.

I was wondering about this line, which is essentially what my code broke the intended behavior of by setting the tag to my own type:

Code: Select all

				var otherCharacter = other.Tag as CharacterController; //Note that this does NOT check for SphereCharacterControllers.  That means it's not safe to multithread with both SphereCharacterControllers and CharacterControllers active.

Perhaps it would be good to have an IUniqueId interface that has an InstanceId which the subsequent sort could use and then the Monitor.Enter/Exit could use the interface instead of the CharacterController. This would allow you to hook in the same interface to SphereController, and I could probably implement the interface in my own objects I was using in the tag.

That being said, it does seem like this problem is a bit more general than it first seemed and perhaps it makes sense to add support for a more general purpose version of this locking logic that doesn't require the tags.

Incidentally, on the subject of tags, I have found it useful to add a uint TagValue adjacent to the existing Tag member. This lets me store app specific data on entities (bitfields) without incurring object overhead, which has had some small value for me (no pun intended).

[Norbo]

The character now locks on an ICharacterTag such that SphereCharacterControllers and CharacterControllers should* be able to coexist. An assert checks the tag for validity to make related issues more obvious too.

*They can't yet due to some sneaky issues I'm still tracking down. Once I get it straightened out I'll post it up.

That being said, it does seem like this problem is a bit more general than it first seemed and perhaps it makes sense to add support for a more general purpose version of this locking logic that doesn't require the tags.

I had some similar inclinations (mental flashes of generalized synchronization groups, configurable interaction behaviors, ...) but it set off my overdesigning alarms pretty quickly. This use case is isolated enough and matches the Tag system pretty well.

Incidentally, on the subject of tags, I have found it useful to add a uint TagValue adjacent to the existing Tag member. This lets me store app specific data on entities (bitfields) without incurring object overhead, which has had some small value for me (no pun intended).

Since such a numerical tag is trivial to add, I'll leave that up to the user

[Norbo]

The development version is now updated: http://bepuphysics.codeplex.com/SourceC ... evelopment

[Danthekilla]

The development version is now updated: http://bepuphysics.codeplex.com/SourceC ... evelopment

Umm this is fantastic...

I literally just gave up on fixing this my self and came to the forums to see if anyone else had issues with using a mix of CharacterController and SphereCharacterController's in a multi threaded simulation.

Also 2 quick questions Norbo, but I put them up here viewtopic.php?f=9&t=1673

[ecosky]

Thanks Norbo!