Hi, I’m trying to implement a behavior in my 2D platformer for dropping down from platforms. I’m making checks in OnCollisionStay2D to see if the player is on a platform that they can drop down from, and then I start a coroutine that should handle the logic.
The logic itself is based on temporarily disabling collision with the given platform, hence the need for OnCollisionStay2D and a coroutine to later re-enable the collision.
The Issue
It works fine for one platform, but if I drop on another platform and trigger the drop down input again before the first coroutine re-enables the collision with the previous platform, the delegate for the first coroutine seems to be changed to the delegate of the new coroutine, causing the first platform collision to remain disabled indefinitely, while the second platform’s collision is enabled twice (once for each coroutine run).
Is that an expected behavior for coroutines? How can I work around it?
I pasted the code I wrote below. The debug logs therein result in the console output from the attached screenshot, in case I try to drop from two+ platforms in the way described above. The 3 colliders triggered are in this Y axis order top to bottom: floating_island_01, top layer collider, and middle layer collider.
private void OnCollisionStay2D(Collision2D collision)
{
// Dropping down from platforms
if (dropDownInput && playerData.IsOneWayPlatform(collision.gameObject.layer))
{
StartCoroutine(HandleDroppingDown_Co(collision));
}
}
private IEnumerator HandleDroppingDown_Co(Collision2D collision)
{
// turn off collision for the character and the platform
Debug.Log($"Disabling collision for {collision.collider.name}");
Physics2D.IgnoreCollision(characterCollider, collision.collider, true);
// add downward thrust to kick-start the descent
rb.AddForce(Vector2.down * playerData.dropDownImpulse, ForceMode2D.Impulse);
// start the timer to re-enable the collision
yield return new WaitForSeconds(playerData.dropCollisionResetSeconds);
Debug.Log($"Enabling collision for {collision.collider.name}");
Physics2D.IgnoreCollision(characterCollider, collision.collider, false);
}
That’s what stumps me. I don’t stop the coroutine directly - I don’t even save the reference to it - I never call StopAllCoroutines in the whole project, and the code above lives directly in the player controller right now, so I’m positive it doesn’t get disabled.
I was thinking something happens to the reference to the coroutine, but the fact that both coroutines finish (as observed by the double re-enabling of the second collider) means either the whole delegate function or its argument gets somehow replaced by the second coroutine.
I’m not a seasoned 2D developer, so it’s a shot in the dark. But I would suspect that the collision data gets reused in the second collision somewhere in the Physics2D. Try to cache the collider before you call the coroutine and use that cached collider. I’m assuming reference types get passed around here. (Again, just an idea)
Looking at your Debug messages, it appears that the Coroutine that runs floating_island_1 doesn’t ever finish. So it’s almost like it’s getting stopped. It looks like Top Layer Collider gets run twice as it is both disabled and enabled twice. This would suggest the first coroutine is getting killed off before it can complete. But this is just a thought based on what I’m seeing in the debug messages.
@ Suggestion is a good one to try, simply assigning collision to a local variable within the method might be enough of a fix.
It may also be better to let the platforms handle their own collision setting instead of the player trying to manage things. So when you drop from a platform, have a script on the platform you can call and let it disable and enable itself. Just a thought.
Having multiple coroutines running in parallel that are started in every frame - and because you’re starting a coroutine in OnCollisionStay, that IS what you’re doing - is gonna have hard-to-predict effects due to overlapping code execution. I would not use a coroutine for this. I would have a script on the platforms that handles their own collision, and they need to store only a time at which they re-enable their collision, which they check in their own Update rather than a coroutine.
Well, I was assuming that the dropDownInput is only true for one frame. Kind of like the .wasPressedThisFrame or the Input.GetKeyDown. If that’s true, there is no new coroutine every frame.
If you’re reusing the Collision2D instance via this setting (and I would highly recommend you have this on as it completely stops GC allocations) then you cannot pass it on beyond the scope of the callback. Take what you need out of it and pass that along instead.
Also, I’d stay away from using OnCollisionStay2D too much as it can hurt performance because it happens so often. A handful here and there won’t make any difference but there are much better options such as using IsTouching to query contacts or even retrieving contacts directly.
I added an integer id to the coroutine so I could get a better idea what magic is going on, and turns out @ and others were on the right track. The moment the collision happens, something within Physics2D reassigns the new collision and all coroutines that haven’t yet finished then reference the same collision at the time of invoking the result.
See how the disabling of Collision #1 references floating_island_01 while enabling of collision #1 references the Top layer collider:
I changed the coroutine to take in the Collider2D instead (the whole collision was redundant anyway) and it started working as expected
(Disregard the warnings, that’s a reminder to fix something else )
That is 100% correct. I’m in the process of transition into a state machine based approach, but this logic is yet to be put in a more sensible place/script.
I was considering the impact of potentially starting plethora of coroutines, but the start of the coroutine is conditioned by 1) user input through the dropDownInput flag and 2) an active collision with a collider. Once it triggers, it immediately disables collisions with this collider, and therefore no more OnCollisionX methods should run, so it shouldn’t be possible for it to go havoc. Probably. Hopefully.
Thanks everyone for your suggestions and feedback! I’m considering this now solved. In the future, I’ll be sure not to use the Collision2D object outside the method that provides it, as @MelvMay advises.
