[Bug] KeyNotFoundException in CheckGround

echtnice · Aug 4, 2021

Hi,

i'm getting a KeyNotFoundException in CheckGround. As I understand the NonAllocCast method, the caller is responsible for calling ResetCombinedRaycastHits once the result of NonAllocCast is not needed anymore. Calling NonAllocCast without calling ResetCombinedRaycastHits could lead to the exception i've hit.

I think there are 2 missing calls to ResetCombinedRaycastHits. Maybe we should just call ResetCombinedRaycastHits in the beginning of NonAllocCast?

Thanks

Code:

diff --git a/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs b/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
index dff566943..d712f950c 100644
--- a/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
+++ b/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
@@ -577,6 +577,7 @@ namespace Opsive.UltimateCharacterController.Character
                         break;
                     }
                 }
+                ResetCombinedRaycastHits();
                 if (!platformCollision) {
                     UpdateMovingPlatformTransform(null, false);
                     return;
@@ -1067,6 +1068,7 @@ namespace Opsive.UltimateCharacterController.Character
                     // Convert back to the global direction for CheckGround.
                     m_MoveDirection = m_Transform.TransformDirection(localMoveDirection);
                 }
+                ResetCombinedRaycastHits();
             }
 
             // Is the character on the ground?

Code:

KeyNotFoundException: The given key was not present in the dictionary.
System.Collections.Generic.Dictionary`2[TKey,TValue].get_Item (TKey key) (at <695d1cc93cca45069c528c15c9fdd749>:0)
Opsive.UltimateCharacterController.Character.CharacterLocomotion.CheckGround () (at Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs:1141)
Opsive.UltimateCharacterController.Character.CharacterLocomotion.DeflectVerticalCollisions () (at Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs:1073)
Opsive.UltimateCharacterController.Character.CharacterLocomotion.UpdatePosition () (at Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs:771)
Opsive.UltimateCharacterController.Character.UltimateCharacterLocomotion.UpdatePosition () (at Assets/Opsive/UltimateCharacterController/Scripts/Character/UltimateCharacterLocomotion.cs:974)

Justin · Aug 5, 2021

Are you able to tell me how to reproduce this error or send me a repro scene? I'd like to look closer into the cause.

echtnice · Aug 13, 2021

Okay even though I am not able to send you a repro scene I took closer look again as I was still experiencieng the problem with the proposed patch above.

My analysis showed that m_CombinedRaycastHits contained holes (items in between are equal m_BlankRaycastHit) and because ResetCombinedRaycastHits() only clears elements until the first m_BlankRaycastHit is found, some items are NOT cleared.

From NonAllocCast

Code:

                    if (m_ColliderCount > 1) {
                        int validHitCount = 0;
                        for (int j = 0; j < localHitCount; ++j) {
                            if (m_ColliderIndexMap.ContainsKey(m_RaycastHits[j])) {
                                continue;
                            }
                            // Ensure the array is large enough.
                            if (hitCount + j>= m_CombinedRaycastHits.Length) {
                                Debug.LogWarning("Warning: The maximum number of collisions has been reached. Consider increasing the CharacterLocomotion MaxCollisionCount value.");
                                continue;
                            }

                            m_ColliderIndexMap.Add(m_RaycastHits[j], i);
                            m_CombinedRaycastHits[hitCount + j] = m_RaycastHits[j];
                            validHitCount += 1;
                        }
                        hitCount += validHitCount;
                    } else {
                        m_CombinedRaycastHits = m_RaycastHits;
                        hitCount += localHitCount;
                    }

So the question is why does m_CombinedRaycastHits contains holes? In NonAllocCast, in case m_ColliderCount > 1. we skip inserting m_RaycastHits[j] into m_ColliderIndexMap if it is already being inserted. But we keep inserting into m_CombinedRaycastHits with the index var j, which is wrong because we might skip inserting and therefore should skip the index j too. Instead of j we should use validHitCount for inserting.

This will result in an continious m_CombinedRaycastHits without holes.

Code:

git a/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs b/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
index a14c5c886..3dba2c9e5 100644
--- a/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
+++ b/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
@@ -1497,13 +1493,13 @@ namespace Opsive.UltimateCharacterController.Character
                                 continue;
                             }
                             // Ensure the array is large enough.
-                            if (hitCount + j >= m_CombinedRaycastHits.Length) {
+                            if (hitCount + validHitCount >= m_CombinedRaycastHits.Length) {
                                 Debug.LogWarning("Warning: The maximum number of collisions has been reached. Consider increasing the CharacterLocomotion MaxCollisionCount value.");
                                 continue;
                             }
 
                             m_ColliderIndexMap.Add(m_RaycastHits[j], i);
-                            m_CombinedRaycastHits[hitCount + j] = m_RaycastHits[j];
+                            m_CombinedRaycastHits[hitCount + validHitCount] = m_RaycastHits[j];
                             validHitCount += 1;
                         }
                         hitCount += validHitCount;

Justin · Aug 15, 2021

Awesome, your change makes a lot of sense. Thanks for digging into it!

funtastyk · Sep 1, 2021

echtnice said:

Okay even though I am not able to send you a repro scene I took closer look again as I was still experiencieng the problem with the proposed patch above.

My analysis showed that m_CombinedRaycastHits contained holes (items in between are equal m_BlankRaycastHit) and because ResetCombinedRaycastHits() only clears elements until the first m_BlankRaycastHit is found, some items are NOT cleared.

From NonAllocCast

Code:

                    if (m_ColliderCount > 1) {
                        int validHitCount = 0;
                        for (int j = 0; j < localHitCount; ++j) {
                            if (m_ColliderIndexMap.ContainsKey(m_RaycastHits[j])) {
                                continue;
                            }
                            // Ensure the array is large enough.
                            if (hitCount + j>= m_CombinedRaycastHits.Length) {
                                Debug.LogWarning("Warning: The maximum number of collisions has been reached. Consider increasing the CharacterLocomotion MaxCollisionCount value.");
                                continue;
                            }

                            m_ColliderIndexMap.Add(m_RaycastHits[j], i);
                            m_CombinedRaycastHits[hitCount + j] = m_RaycastHits[j];
                            validHitCount += 1;
                        }
                        hitCount += validHitCount;
                    } else {
                        m_CombinedRaycastHits = m_RaycastHits;
                        hitCount += localHitCount;
                    }

So the question is why does m_CombinedRaycastHits contains holes? In NonAllocCast, in case m_ColliderCount > 1. we skip inserting m_RaycastHits[j] into m_ColliderIndexMap if it is already being inserted. But we keep inserting into m_CombinedRaycastHits with the index var j, which is wrong because we might skip inserting and therefore should skip the index j too. Instead of j we should use validHitCount for inserting.

This will result in an continious m_CombinedRaycastHits without holes.

Code:

git a/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs b/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
index a14c5c886..3dba2c9e5 100644
--- a/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
+++ b/Assets/Opsive/UltimateCharacterController/Scripts/Character/CharacterLocomotion.cs
@@ -1497,13 +1493,13 @@ namespace Opsive.UltimateCharacterController.Character
                                 continue;
                             }
                             // Ensure the array is large enough.
-                            if (hitCount + j >= m_CombinedRaycastHits.Length) {
+                            if (hitCount + validHitCount >= m_CombinedRaycastHits.Length) {
                                 Debug.LogWarning("Warning: The maximum number of collisions has been reached. Consider increasing the CharacterLocomotion MaxCollisionCount value.");
                                 continue;
                             }
 
                             m_ColliderIndexMap.Add(m_RaycastHits[j], i);
-                            m_CombinedRaycastHits[hitCount + j] = m_RaycastHits[j];
+                            m_CombinedRaycastHits[hitCount + validHitCount] = m_RaycastHits[j];
                             validHitCount += 1;
                         }
                         hitCount += validHitCount;

Everything works great, thanks a lot =)

[Bug] KeyNotFoundException in CheckGround

echtnice

Member

Justin

Administrator

echtnice

Member

Justin

Administrator

funtastyk

New member