Is it possible for OnCollisionEnter to be called twice for the same collision?

I've got a breakout game that I'm using to learn about Unity and a couple of times now I've seen odd behavior. Specifically I get a "game over, you win" message even though there is still one brick left on the screen. Now, to determine the end of the game I am keeping an int with a count of the number of remaining bricks, and this count is decremented in the brick objects OnCollisionEnter method when the ball collides with the brick. (This method decrements the brick count, increments the score, and destroys the brick object.) So this suggests to me that sometimes OnCollisionEnter is being called twice for the same brick object. Is that possible?

 

May we see a code of your OnCollisionEnter? You could also try OnTriggerEnter as well.. OnCollisionEnter may be acting up because when your thing collides. It might have a very very slight bump from colliding and detecting two hits before the frame is complete.

 

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class Brick : MonoBehaviour {
5.  
6.     // Sometimes it seems that a brick can have OnCollisionEnter called twice, so this is a guard
7.     // to make sure we don't accidentally double decrement brick_count.
8.     private bool destroyed = false;
9.  
10.     public void OnCollisionEnter( Collision col ) {
11.         if ( !destroyed ) {
12.             destroyed = true;
13.             Destroy( gameObject, 0.1f );
14.             BrickBlock.brick_count--;
15.             GameController.instance.increment_score();
16.         }
17.     }
18.  
19. }

That's the whole of the code attached to the brick object, which is a GameObject with a custom Mesh, Mesh Renderer, Animator (Hmm? Why is that there?), and a Box Collider.

All the code referencing the "destroyed" bool was added tonight as an attempt to address the problem.

 

Yeah I have a feeling it's doing what I mentioned.
What's the problem with using the destroyed bool? Is that not working either? Because unless you use OnTriggerEnter with Collider instead of Collision. it will detect the most minute collisions. So if it boucnes even 0.00000000000000001 and hits again, it will detect that collision probably before the frame even ends.

So if the bool setting isn't working. I recommend doing OnTriggerEnger with Collider instead of Collision.
then just do

if(col.gameObject.tag == "SomeTag"){ // do your code }.

(in my observation) using triggers is more reliable then using OnCollision stuff when it comes to doing instant actions.
I rarely ever use OnCollisionEnter for anything. It's more for collision based stuff. Such as OnCollisionEnter(Keep player parented to this object) to move with a platform or something.

Using Triggers on the other hand, it can only detect that one single hit as it will then trigger the hit instantly and not have to worry about bounces.

 

Hmm. I don't think I can use a trigger here since I'm relying on the physics engine to bounce the ball on collisions. But it looks like I may be able to put the brick_count and score update in OnDestroy instead - I just tested that and when Destroy is called more than once on the same object OnDestroy seems to still only be called once. That might be a more elegant solution that the boolean guard, which seems like it could still fail if the right race condition is hit. (e.g. the first OnCollisionEnter enters the if block and then the CPU context switches to the second OnCollisionEnter before the value of destroyed is updated.)

 

Add a Debug.Log in that code you listed above to print out the name of that particular brick game object and other unique information such as its position. Then you'll know for certain if it is an issue of the collision happening twice.

 

You can always use a Trigger and collider at the same time. The trigger detects what it needs and the regular collisions still bounce it... (I have done it at least in past versions). You may have to adjust the trigger/collision collider range..

Have it where the Trigger one is further out (to deduct -1 from the brick count). Then have the Collision Enter actually destroy the brick.

 

Why not just store the state of each brick in an array and then loop-check the status of each entry in the array to determine the win state? Unless you have a playfield of 10,000,000x10,000,000 the performance difference is going to be negligible.

 

Because the programming snob in me says "that is NOT elegant".

It's a thought, though, and I've considered doing something like keeping a list of the bricks and checking for when the list is empty. A similar idea, but I'd still be only checking a count variable instead of iterating over every element.

 

I would almost certainly do this. On top of counting them for you it gives you an easy place to manage issues like removing the same brick twice because you can easily tell when it's happened.

Also, I don't know about your game, but is it possible that in the future it'll be valid to have multiple simultaneous contacts between the same pair of objects? Eg: maybe you add an L-shaped brick and the ball collides at an inner corner. The list approach handles this natively, where you'd have to add an intermediary step to handle it with a manual counter.

Plus, it means you've got a single collection storing all of your bricks, which is handy for all sorts of things.

Out of interest, what don't you like about it? Is it just looping to count? I presume you don't have to do that very often and also that you don't have especially many bricks, so I wouldn't worry about it personally.

 

Since you're setting the destroyed variable to true and using that in a conditional check, I don't see how the collision occurring twice would be causing your issue, as the second collision would simply be ignored.

So I think it must be something else causing this.

Are you sure your brick count is correctly set to match the number of bricks in the level?

 

Destroy doesn't actually happen until the end of a frame. OnCollision happens within the FixedUpdate cycle. Its technically possible to get multiple collisions before the object is destroyed. But I don't think this is what's happening in your case.

Honestly I would just go with an collection of objects and check if the collection is empty.

Is this intermittent? That would suggest an error with your collision handling.

But if the error happens every time I would suggest you are simply fence posting.

 

For a start, I'd advise to strangle inner programming snob.

The problem with OnCollision/Trigger Enter/Exit is that they're acting, well, strangely. Enter/Exit should form pairs (Exit for each Enter), except that they don't do that, and I've heard that it is possible to miss "Enter" even entirely if an object teleports inside the collider. IIRC there's some sort of shenanigans going on regarding actual collider being removed from physics system as well. I think it doesn't happen at the moment when you destroy the object, but a bit later.

The way I'd go about it would be to set up list or hash set of bricks, remove bricks from the list/set when they're destroyed, and gameover when the list/set is empty.

Regarding "destroyed" variable... another way to go about it is to disable game (setActive(false)) object before destroying it.This should disable its colliders as well... however "destroyed" variable is good enough.

Also, the object might also be receiving collisions from something other than the ball, unless you set up layers very carefully. In this case you'll receive multiple collision events, which will result in premature gameover. That's something you might keep in mind.

 

Just habit, really. I'm used to working with very large data sets where looping over every element simply isn't a practical idea.

 

So:

Code (CSharp):

1. Destroy( gameObject, 0.1f );
2. BrickBlock.brick_count--;
3. GameController.instance.increment_score();

I'm assuming that the BrickBlock.brickCount variable is checked to see if you should show the you win screen? You're not by any chance pausing the game through Time.timeScale = 0 when you show that screen?

It's there because Unity always adds an animator when you drag a model (.fbx/.blend) into the scene. It's rather annoying, really!

 

No, I'm not pausing the game. In fact, doing something like that is on my todo list, because right now the controls remain active even when no game is actively being played. Is there something I need to be aware of if I set the timescale to 0?

 

If you had done that, the Destroy(gameObject, 0.1f) would have caused the final brick to not be destroyed due to the 0.1f second not passing - the time argument on Destroy is timeScale-dependent, as you'd expect. So that was what I was guessing was happening!

You still haven't answered a very important question; is this happening every time, or just once in a while? It's a big difference for what might be going on. Posting the code that triggers the game over screen would also be helpful!

Also, just caught something in your post:

The fixed timestep (in which OnCollisionEnter happens) runs in a single thread, so that won't happen. In fact, the entire core Unity API is not thread safe. You're not hitting a race condition.

Now, the engine itself uses a lot of multithreading in the background, but the user-space callbacks are all single-threaded. You can also use threads as much as you want, you just can't touch the engine in there - which means GameObjects and their components, mostly.

 

No it's not happening every time. As I said in the original post, I've only seen it a couple of times, and I can't reproduce it.

I don't have the code with mee right now, but I'll get it when I get home tonight.

 

After thinking about this some, I think I'll reimplement the brick list as a Dictionary keyed on the instanceID of the brick objects. That may be overkill in this case, but I'm also learning C# as I go along, so dinking with the collection classes seems like good exercise. This way I don't have to worry about multiple collision events since the subsequent events would find the object has already been removed from the dictionary and there's nothing more to do.

 

I would check to make sure you don't have duplicated objects somewhere, or maybe the same script twice on a single brick. Also do a search for BrickBlock.brickCount in your scripts to find out if you are accidentally decrementing it somewhere else.

I just don't see how the collision being detected twice would be causing this issue; again, the second detection wouldn't do anything since in the first detection you set the destroyed variable to true . . . it doesn't matter that the actual destruction of the brick doesn't happen until the end of the frame.

 

You misunderstood - I added the destroyed variable as an attempt to solve the problem. I have not seen the problem since then, but at the same time I never had a reliable way to reproduce the bug, either, so not seeing it isn't proof that it is gone.

And I already searched for where brick_count is accessed - that was the first thing I did, actually.

I kind of think that N1warhead has the right explanation: that more than one collision is occurring in the same frame. What I think might be happening is this: the bricks are not perfectly rectangular, they have beveled edges, like this:

Code (CSharp):

1. |     ||     ||     |
2. \_____/\_____/\_____/
3.  

I think the ball is hitting right on the corner of one brick, bouncing over and hitting the brick next to it, and then bouncing back to hit the first brick a second time. That would be a pretty rare occurrence but it would explain what is going on.

 

Ah okay, I must have missed where you said adding the boolean guard caused the issue to disappear (or at least, appear to disappear since it never reliably occurred in the first place).

I personally don't see any issue with the boolean guard, though I would change the code so the brick itself is not decrementing the brick count or checking for the "win" condition (brick count equal to 0). Thinking of the brick as a separate entity, it just doesn't make sense that it should have knowledge of the rest of your game like it currently does.

You might consider using a messenger/notification system. Keep the guard in place and when the brick should be destroyed, destroy it and have it broadcast the message that it has been destroyed. Then another system will listen for brick destruction messages and react accordingly once it's count of all bricks reaches 0.

With this system you could easily add more systems that listen for the brick destruction and react accordingly (such as a sound system).

This is just my personal opinion without knowing much about the rest of your code. Lots of other great suggestions in this thread. Good luck!

 

Heck yeah!

I use this kind of approach all the time. As the most relevant example, I made a match-3 game a while ago where the contents of the board is just data, and messages get raised whenever something "interesting" happens. I then have separate components attached to the board which listen for those messages and implement literally whatever rules I want.

I made a cut-down WebGL demo of that game a couple of weeks ago, and it took me less than an hour to make two new sets of rules for the game, because almost none of the code is aware of or affected by the fact that there even are rules being applied.

As another example, I made a puzzle game before that where I decided I didn't like the UI, which is of course where a lot of the controls were being applied. I just deleted it from the scene and made a new one. The new one raised the same messages as the old one for inputs, and listened for the same state change messages... so I didn't have to change a single piece of code (aside from the new UI bits I made, of course).

I originally thought that having messages flying around randomly would make exceptionally complex, hard to understand and difficult to maintain software. That's what intuition says - if there's not a nicely explicit order of execution then how do I remain in control? Isn't it going to just descend into a soup of random calls all over the joint? In reality, though, once I got used to it - which didn't take long - I found that the opposite was true. Less code runs, and it runs less often, and when it's designed well it's easier to follow, and your software ends up being composed of self-contained modular bits that are individually nice and easy to maintain.

The downside is that you can't fully follow execution paths just by looking at code, because you can't tell what is listening for what without also being aware of what's hooked up either dynamically or in the Editor. In practice I don't find this to be much of an issue, but I have the privilege of being able to be very disciplined about how I do things.