diff --git a/binaries/data/mods/public/simulation/data/pathfinder.xml b/binaries/data/mods/public/simulation/data/pathfinder.xml index 17397a44dd..eb912f1c1c 100644 --- a/binaries/data/mods/public/simulation/data/pathfinder.xml +++ b/binaries/data/mods/public/simulation/data/pathfinder.xml @@ -11,7 +11,7 @@ pathfinding 2 1.0 - 1.0 + 0.8 pathfinding diff --git a/source/simulation2/components/CCmpObstructionManager.cpp b/source/simulation2/components/CCmpObstructionManager.cpp index dc122aa3f9..7bca40c463 100644 --- a/source/simulation2/components/CCmpObstructionManager.cpp +++ b/source/simulation2/components/CCmpObstructionManager.cpp @@ -995,8 +995,12 @@ void CCmpObstructionManager::GetUnitsOnObstruction(const ObstructionSquare& squa if (rasterizedRects.find(shape.clearance) == rasterizedRects.end()) { + // Wraitii 31/10/15: check out helpers/Rasterize.cpp for more info, but + // a change in the long-range pathfinder rasterization to fix stuck units + // requires the foundation code to be more restrictive, thus adding the substraction + // of Pathfinding::NAVCELL_SIZE. Remove it if the problem in Rasterize.cpp is ever fixed. SimRasterize::Spans& newSpans = rasterizedRects[shape.clearance]; - SimRasterize::RasterizeRectWithClearance(newSpans, square, shape.clearance, Pathfinding::NAVCELL_SIZE); + SimRasterize::RasterizeRectWithClearance(newSpans, square, shape.clearance-Pathfinding::NAVCELL_SIZE, Pathfinding::NAVCELL_SIZE); } SimRasterize::Spans& spans = rasterizedRects[shape.clearance]; diff --git a/source/simulation2/components/CCmpPathfinder_Vertex.cpp b/source/simulation2/components/CCmpPathfinder_Vertex.cpp index 8ed9d22fb1..88fa8a3b20 100644 --- a/source/simulation2/components/CCmpPathfinder_Vertex.cpp +++ b/source/simulation2/components/CCmpPathfinder_Vertex.cpp @@ -127,7 +127,7 @@ struct EdgeAA // When computing vertexes to insert into the search graph, // add a small delta so that the vertexes of an edge don't get interpreted // as crossing the edge (given minor numerical inaccuracies) -static const entity_pos_t EDGE_EXPAND_DELTA = entity_pos_t::FromInt(1)/4; +static const entity_pos_t EDGE_EXPAND_DELTA = entity_pos_t::FromInt(1)/16; /** * Check whether a ray from 'a' to 'b' crosses any of the edges. diff --git a/source/simulation2/components/CCmpUnitMotion.cpp b/source/simulation2/components/CCmpUnitMotion.cpp index f8fb99a6bf..162192ffd1 100644 --- a/source/simulation2/components/CCmpUnitMotion.cpp +++ b/source/simulation2/components/CCmpUnitMotion.cpp @@ -1042,7 +1042,9 @@ void CCmpUnitMotion::PlanNextStep(const CFixedVector2D& pos) return; CmpPtr cmpPathfinder(GetSystemEntity()); - + if (!cmpPathfinder) + return; + const Waypoint& nextPoint = m_LongPath.m_Waypoints.back(); // The next step was obstructed the last time we checked; also check that diff --git a/source/simulation2/helpers/HierarchicalPathfinder.cpp b/source/simulation2/helpers/HierarchicalPathfinder.cpp index 07661990e2..995c3d9a9b 100644 --- a/source/simulation2/helpers/HierarchicalPathfinder.cpp +++ b/source/simulation2/helpers/HierarchicalPathfinder.cpp @@ -352,8 +352,7 @@ void HierarchicalPathfinder::Update(Grid* grid, const Grid& dir if (std::find(processedChunks.begin(), processedChunks.end(), chunkID) == processedChunks.end()) { - // iterate over a std::map - for (auto& passClassMask : m_PassClassMasks) + for (const std::pair& passClassMask : m_PassClassMasks) { pass_class_t passClass = passClassMask.second; Chunk& a = m_Chunks[passClass].at(chunkID.second*m_ChunksW + chunkID.first); @@ -366,8 +365,7 @@ void HierarchicalPathfinder::Update(Grid* grid, const Grid& dir // TODO: Also be clever with edges m_Edges.clear(); - // iterate over a std::map - for (auto& passClassMask : m_PassClassMasks) + for (const std::pair& passClassMask : m_PassClassMasks) { pass_class_t passClass = passClassMask.second; EdgesMap& edges = m_Edges[passClass]; diff --git a/source/simulation2/helpers/Pathfinding.h b/source/simulation2/helpers/Pathfinding.h index 5ce7818b75..28865e3044 100644 --- a/source/simulation2/helpers/Pathfinding.h +++ b/source/simulation2/helpers/Pathfinding.h @@ -310,6 +310,12 @@ public: { m_Clearance = node.GetChild("Clearance").ToFixed(); + /* According to Philip who designed the original doc (in docs/pathfinder.pdf), + * clearance should usually be integer to ensure consistent behavior when rasterizing + * the passability map. + * This seems doubtful to me and my pathfinder fix makes having a clearance of 0.8 quite convenient + * so I comment out this check, but leave it here for the purpose of documentation should a bug arise. + if (!(m_Clearance % Pathfinding::NAVCELL_SIZE).IsZero()) { // If clearance isn't an integer number of navcells then we'll @@ -317,7 +323,7 @@ public: // by clearance, vs expanding static obstructions by clearance LOGWARNING("Pathfinder passability class has clearance %f, should be multiple of %f", m_Clearance.ToFloat(), Pathfinding::NAVCELL_SIZE.ToFloat()); - } + }*/ } else m_Clearance = fixed::Zero(); diff --git a/source/simulation2/helpers/Rasterize.cpp b/source/simulation2/helpers/Rasterize.cpp index 5d33fbcb08..7a32efe832 100644 --- a/source/simulation2/helpers/Rasterize.cpp +++ b/source/simulation2/helpers/Rasterize.cpp @@ -25,9 +25,26 @@ void SimRasterize::RasterizeRectWithClearance(Spans& spans, const ICmpObstructionManager::ObstructionSquare& shape, entity_pos_t clearance, entity_pos_t cellSize) { + // A long-standing issue with the pathfinding has been that the long-range one + // uses a AA navcell grid, while the short-range uses an accurate vector representation. + // This means we could get paths accepted by one but not both pathfinders. + // Since the new pathfinder, the short-range pathfinder's representation was usually + // encompassing the rasterisation of the long-range one for a building. + // This means that we could never get quite as close as the long-range pathfinder wanted. + // This could mean units tried going through impassable paths. + // To fix this, we need to make sure that the short-range pathfinder is always mostly + // included in the rasterisation. The easiest way is to rasterise more, thus this variable + // Since this is a very complicated subject, check out logs on 31/10/2015 for more detailled info. + // or ask wraitii about it. + // If the short-range pathfinder is sufficiently changed, this could become unnecessary and thus removed. + // A side effect is that the basic clearance has been set to 0.8, so removing this line should be done + // in parallel with setting clearance back to 1 for the default passability class (though this isn't strictly necessary). + // Also: the code detecting foundation obstruction in CcmpObstructionManager had to be changed similarly. + entity_pos_t rasterClearance = clearance + entity_pos_t::FromInt(1); + // Get the bounds of cells that might possibly be within the shape // (We'll then test each of those cells more precisely) - CFixedVector2D halfSize(shape.hw + clearance, shape.hh + clearance); + CFixedVector2D halfSize(shape.hw + rasterClearance, shape.hh + rasterClearance); CFixedVector2D halfBound = Geometry::GetHalfBoundingBox(shape.u, shape.v, halfSize); i16 i0 = ((shape.x - halfBound.X) / cellSize).ToInt_RoundToNegInfinity(); i16 j0 = ((shape.z - halfBound.Y) / cellSize).ToInt_RoundToNegInfinity(); @@ -38,11 +55,11 @@ void SimRasterize::RasterizeRectWithClearance(Spans& spans, return; // empty bounds - this shouldn't happen spans.reserve(j1 - j0); - + for (i16 j = j0; j < j1; ++j) { - // Find the min/max range of cells that are strictly inside the square+clearance. - // (Since the square+clearance is a convex shape, we can just test each + // Find the min/max range of cells that are strictly inside the square+realClearance. + // (Since the square+realClearance is a convex shape, we can just test each // corner of each cell is inside the shape.) // (TODO: This potentially does a lot of redundant work.) i16 spanI0 = std::numeric_limits::max(); @@ -51,28 +68,28 @@ void SimRasterize::RasterizeRectWithClearance(Spans& spans, { if (Geometry::DistanceToSquare( CFixedVector2D(cellSize*i, cellSize*j) - CFixedVector2D(shape.x, shape.z), - shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > clearance) + shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > rasterClearance) { continue; } if (Geometry::DistanceToSquare( CFixedVector2D(cellSize*(i+1), cellSize*j) - CFixedVector2D(shape.x, shape.z), - shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > clearance) + shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > rasterClearance) { continue; } if (Geometry::DistanceToSquare( CFixedVector2D(cellSize*i, cellSize*(j+1)) - CFixedVector2D(shape.x, shape.z), - shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > clearance) + shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > rasterClearance) { continue; } if (Geometry::DistanceToSquare( CFixedVector2D(cellSize*(i+1), cellSize*(j+1)) - CFixedVector2D(shape.x, shape.z), - shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > clearance) + shape.u, shape.v, CFixedVector2D(shape.hw, shape.hh), true) > rasterClearance) { continue; }