From dff694f0f06c8c04dd9a21e3cd2ac7938a3b5e85 Mon Sep 17 00:00:00 2001 From: Ykkrosh Date: Sun, 4 Jul 2010 17:19:38 +0000 Subject: [PATCH] Fix units spawning on top of each other. Add type-safety to prevent that kind of bug happening again. This was SVN commit r7691. --- .../simulation/components/TrainingQueue.js | 1 + .../simulation2/components/CCmpFootprint.cpp | 10 +++++++--- .../components/CCmpObstruction.cpp | 18 +++++++++--------- .../components/CCmpObstructionManager.cpp | 19 ++++++++++--------- .../simulation2/components/CCmpUnitMotion.cpp | 8 ++++---- .../components/ICmpObstructionManager.h | 14 +++++++++++--- 6 files changed, 42 insertions(+), 28 deletions(-) diff --git a/binaries/data/mods/public/simulation/components/TrainingQueue.js b/binaries/data/mods/public/simulation/components/TrainingQueue.js index ac414aedd6..d3f0ed16cf 100644 --- a/binaries/data/mods/public/simulation/components/TrainingQueue.js +++ b/binaries/data/mods/public/simulation/components/TrainingQueue.js @@ -170,6 +170,7 @@ TrainingQueue.prototype.SpawnUnits = function(templateName, count) // What should we do here? // For now, just move the unit into the middle of the building where it'll probably get stuck pos = cmpPosition.GetPosition(); + warn("Can't find free space to spawn trained unit"); } var cmpNewPosition = Engine.QueryInterface(ent, IID_Position); diff --git a/source/simulation2/components/CCmpFootprint.cpp b/source/simulation2/components/CCmpFootprint.cpp index a8cd0d9e36..6d2649a798 100644 --- a/source/simulation2/components/CCmpFootprint.cpp +++ b/source/simulation2/components/CCmpFootprint.cpp @@ -135,11 +135,15 @@ public: return error; entity_pos_t spawnedRadius; + ICmpObstructionManager::tag_t spawnedTag; CmpPtr cmpSpawnedObstruction(GetSimContext(), spawned); if (!cmpSpawnedObstruction.null()) + { spawnedRadius = cmpSpawnedObstruction->GetUnitRadius(); - // else zero + spawnedTag = cmpSpawnedObstruction->GetObstruction(); + } + // else use zero radius // The spawn point should be far enough from this footprint to fit the unit, plus a little gap entity_pos_t clearance = spawnedRadius + entity_pos_t::FromInt(2); @@ -162,7 +166,7 @@ public: CFixedVector3D pos (initialPos.X + s.Multiply(radius), fixed::Zero(), initialPos.Z + c.Multiply(radius)); - SkipTagObstructionFilter filter(spawned); // ignore collisions with the spawned entity + SkipTagObstructionFilter filter(spawnedTag); // ignore collisions with the spawned entity if (!cmpObstructionManager->TestUnitShape(filter, pos.X, pos.Z, spawnedRadius)) return pos; // this position is okay, so return it } @@ -212,7 +216,7 @@ public: { CFixedVector2D pos (center + dir*i); - SkipTagObstructionFilter filter(spawned); // ignore collisions with the spawned entity + SkipTagObstructionFilter filter(spawnedTag); // ignore collisions with the spawned entity if (!cmpObstructionManager->TestUnitShape(filter, pos.X, pos.Y, spawnedRadius)) return CFixedVector3D(pos.X, fixed::Zero(), pos.Y); // this position is okay, so return it } diff --git a/source/simulation2/components/CCmpObstruction.cpp b/source/simulation2/components/CCmpObstruction.cpp index 4de295d12f..733968a929 100644 --- a/source/simulation2/components/CCmpObstruction.cpp +++ b/source/simulation2/components/CCmpObstruction.cpp @@ -102,7 +102,7 @@ public: else m_Active = true; - m_Tag = 0; + m_Tag = ICmpObstructionManager::tag_t(); m_Moving = false; } @@ -133,18 +133,18 @@ public: const CMessagePositionChanged& data = static_cast (msg); - if (!data.inWorld && !m_Tag) + if (!data.inWorld && !m_Tag.valid()) break; // nothing needs to change CmpPtr cmpObstructionManager(context, SYSTEM_ENTITY); if (cmpObstructionManager.null()) break; - if (data.inWorld && m_Tag) + if (data.inWorld && m_Tag.valid()) { cmpObstructionManager->MoveShape(m_Tag, data.x, data.z, data.a); } - else if (data.inWorld && !m_Tag) + else if (data.inWorld && !m_Tag.valid()) { // Need to create a new pathfinder shape: if (m_Type == STATIC) @@ -152,10 +152,10 @@ public: else m_Tag = cmpObstructionManager->AddUnitShape(data.x, data.z, m_Size0, m_Moving); } - else if (!data.inWorld && m_Tag) + else if (!data.inWorld && m_Tag.valid()) { cmpObstructionManager->RemoveShape(m_Tag); - m_Tag = 0; + m_Tag = ICmpObstructionManager::tag_t(); } break; } @@ -164,7 +164,7 @@ public: const CMessageMotionChanged& data = static_cast (msg); m_Moving = !data.speed.IsZero(); - if (m_Tag && m_Type == UNIT) + if (m_Tag.valid() && m_Type == UNIT) { CmpPtr cmpObstructionManager(context, SYSTEM_ENTITY); if (cmpObstructionManager.null()) @@ -175,14 +175,14 @@ public: } case MT_Destroy: { - if (m_Tag) + if (m_Tag.valid()) { CmpPtr cmpObstructionManager(context, SYSTEM_ENTITY); if (cmpObstructionManager.null()) break; cmpObstructionManager->RemoveShape(m_Tag); - m_Tag = 0; + m_Tag = ICmpObstructionManager::tag_t(); } break; } diff --git a/source/simulation2/components/CCmpObstructionManager.cpp b/source/simulation2/components/CCmpObstructionManager.cpp index 408a221736..612090f5e9 100644 --- a/source/simulation2/components/CCmpObstructionManager.cpp +++ b/source/simulation2/components/CCmpObstructionManager.cpp @@ -34,11 +34,12 @@ // Externally, tags are opaque non-zero positive integers. // Internally, they are tagged (by shape) indexes into shape lists. // idx must be non-zero. -#define TAG_IS_UNIT(tag) (((tag) & 1) == 0) -#define TAG_IS_STATIC(tag) (((tag) & 1) == 1) -#define UNIT_INDEX_TO_TAG(idx) (((idx) << 1) | 0) -#define STATIC_INDEX_TO_TAG(idx) (((idx) << 1) | 1) -#define TAG_TO_INDEX(tag) ((tag) >> 1) +#define TAG_IS_VALID(tag) ((tag).valid()) +#define TAG_IS_UNIT(tag) (((tag).n & 1) == 0) +#define TAG_IS_STATIC(tag) (((tag).n & 1) == 1) +#define UNIT_INDEX_TO_TAG(idx) tag_t(((idx) << 1) | 0) +#define STATIC_INDEX_TO_TAG(idx) tag_t(((idx) << 1) | 1) +#define TAG_TO_INDEX(tag) ((tag).n >> 1) /** * Internal representation of axis-aligned sometimes-square sometimes-circle shapes for moving units @@ -152,7 +153,7 @@ public: virtual void MoveShape(tag_t tag, entity_pos_t x, entity_pos_t z, entity_angle_t a) { - debug_assert(tag); + debug_assert(TAG_IS_VALID(tag)); if (TAG_IS_UNIT(tag)) { @@ -179,7 +180,7 @@ public: virtual void SetUnitMovingFlag(tag_t tag, bool moving) { - debug_assert(tag && TAG_IS_UNIT(tag)); + debug_assert(TAG_IS_VALID(tag) && TAG_IS_UNIT(tag)); if (TAG_IS_UNIT(tag)) { @@ -190,7 +191,7 @@ public: virtual void RemoveShape(tag_t tag) { - debug_assert(tag); + debug_assert(TAG_IS_VALID(tag)); if (TAG_IS_UNIT(tag)) m_UnitShapes.erase(TAG_TO_INDEX(tag)); @@ -202,7 +203,7 @@ public: virtual ObstructionSquare GetObstruction(tag_t tag) { - debug_assert(tag); + debug_assert(TAG_IS_VALID(tag)); if (TAG_IS_UNIT(tag)) { diff --git a/source/simulation2/components/CCmpUnitMotion.cpp b/source/simulation2/components/CCmpUnitMotion.cpp index e6eea80cd9..d2288c3093 100644 --- a/source/simulation2/components/CCmpUnitMotion.cpp +++ b/source/simulation2/components/CCmpUnitMotion.cpp @@ -557,7 +557,7 @@ bool CCmpUnitMotion::MoveToAttackRange(entity_id_t target, entity_pos_t minRange if (cmpObstructionManager.null()) return false; - ICmpObstructionManager::tag_t tag = 0; + ICmpObstructionManager::tag_t tag; CmpPtr cmpObstruction(GetSimContext(), target); if (!cmpObstruction.null()) @@ -590,7 +590,7 @@ bool CCmpUnitMotion::MoveToAttackRange(entity_id_t target, entity_pos_t minRange const entity_pos_t goalDelta = entity_pos_t::FromInt(CELL_SIZE)/4; // for extending the goal outwards/inwards a little bit - if (tag) + if (tag.valid()) { ICmpObstructionManager::ObstructionSquare obstruction = cmpObstructionManager->GetObstruction(tag); @@ -723,7 +723,7 @@ bool CCmpUnitMotion::IsInAttackRange(entity_id_t target, entity_pos_t minRange, if (cmpObstructionManager.null()) return false; - ICmpObstructionManager::tag_t tag = 0; + ICmpObstructionManager::tag_t tag; CmpPtr cmpObstruction(GetSimContext(), target); if (!cmpObstruction.null()) @@ -731,7 +731,7 @@ bool CCmpUnitMotion::IsInAttackRange(entity_id_t target, entity_pos_t minRange, entity_pos_t distance; - if (tag) + if (tag.valid()) { ICmpObstructionManager::ObstructionSquare obstruction = cmpObstructionManager->GetObstruction(tag); diff --git a/source/simulation2/components/ICmpObstructionManager.h b/source/simulation2/components/ICmpObstructionManager.h index eac764c67c..d9e312d0df 100644 --- a/source/simulation2/components/ICmpObstructionManager.h +++ b/source/simulation2/components/ICmpObstructionManager.h @@ -55,9 +55,17 @@ class ICmpObstructionManager : public IComponent { public: /** - * External identifiers for shapes. Valid tags are guaranteed to be non-zero. + * External identifiers for shapes. + * (This is a struct rather than a raw u32 for type-safety.) */ - typedef u32 tag_t; + struct tag_t + { + tag_t() : n(0) {} + explicit tag_t(u32 n) : n(n) {} + bool valid() { return n != 0; } + + u32 n; + }; /** * Register a static shape. @@ -233,7 +241,7 @@ class SkipTagObstructionFilter : public IObstructionTestFilter ICmpObstructionManager::tag_t m_Tag; public: SkipTagObstructionFilter(ICmpObstructionManager::tag_t tag) : m_Tag(tag) {} - virtual bool Allowed(ICmpObstructionManager::tag_t tag, bool UNUSED(moving)) const { return tag != m_Tag; } + virtual bool Allowed(ICmpObstructionManager::tag_t tag, bool UNUSED(moving)) const { return tag.n != m_Tag.n; } }; #endif // INCLUDED_ICMPOBSTRUCTIONMANAGER