From fe283b547a9bfb82eb8ac1c7152d7183896e25d1 Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Tue, 23 Sep 2025 20:31:55 +0100 Subject: [PATCH 1/5] refactor(pathfinder): Simplify and improve readability of Pathfinder::checkDestination (#1645) --- .../Source/GameLogic/AI/AIPathfind.cpp | 88 +++++++++-------- .../Source/GameLogic/AI/AIPathfind.cpp | 95 +++++++++++-------- 2 files changed, 106 insertions(+), 77 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 9967a65427..fc1d65aa81 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -4339,49 +4339,63 @@ Bool Pathfinder::checkDestination(const Object *obj, Int cellX, Int cellY, Pathf for (i=cellX-iRadius; iisAircraftGoal()) continue; - if (cell->getGoalAircraft()==objID) continue; - return false; - } - if (cell->getType()==PathfindCell::CELL_OBSTACLE) { - if (cell->isObstaclePresent( ignoreId )) - continue; - return false; + if (!cell) { + return false; // off the map, so can't place here. + } + + if (checkForAircraft) { + if (!cell->isAircraftGoal()) { + continue; } - if (cell->getFlags() == PathfindCell::NO_UNITS) { - continue; // Nobody is here, so it's ok. + if (cell->getGoalAircraft() == objID) { + continue; } - ObjectID goalUnitID = cell->getGoalUnit(); - if (goalUnitID==objID) { - continue; // we got it. - } else if (ignoreId==goalUnitID) { - continue; // we are ignoring it. - } else if (goalUnitID!=INVALID_ID) { - if (obj==NULL) { - return false; - } - Object *unit = TheGameLogic->findObjectByID(goalUnitID); - if (unit) { - // order matters: we want to know if I consider it to be an ally, not vice versa - if (obj->getRelationship(unit) == ALLIES) { - return false; // Don't usurp your allies goals. jba. - } - if (cell->getFlags()==PathfindCell::UNIT_PRESENT_FIXED) { - Bool canCrush = obj->canCrushOrSquish(unit, TEST_CRUSH_OR_SQUISH); - if (!canCrush) { - return false; // Don't move to an occupied cell. - } - } - } + return false; + } + + if (cell->getType()==PathfindCell::CELL_OBSTACLE) { + if (cell->isObstaclePresent( ignoreId )) + continue; + return false; + } + + if (cell->getFlags() == PathfindCell::NO_UNITS) { + continue; // Nobody is here, so it's ok. + } + + ObjectID goalUnitID = cell->getGoalUnit(); + if (goalUnitID == objID) { + continue; // we got it. + } + + if (goalUnitID == ignoreId) { + continue; // we are ignoring it. + } + + if (goalUnitID == INVALID_ID) { + continue; + } + + if (!obj) { + return false; + } + Object *unit = TheGameLogic->findObjectByID(goalUnitID); + if (!unit) { + continue; + } + + // order matters: we want to know if I consider it to be an ally, not vice versa + if (obj->getRelationship(unit) == ALLIES) { + return false; // Don't usurp your allies goals. jba. + } + if (cell->getFlags()==PathfindCell::UNIT_PRESENT_FIXED) { + Bool canCrush = obj->canCrushOrSquish(unit, TEST_CRUSH_OR_SQUISH); + if (!canCrush) { + return false; // Don't move to an occupied cell. } - } else { - return false; // off the map, so can't place here. } } } - return true; } diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 93976cd80f..c4cc68738b 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -4623,52 +4623,67 @@ Bool Pathfinder::checkDestination(const Object *obj, Int cellX, Int cellY, Pathf for (i=cellX-iRadius; iisAircraftGoal()) continue; - if (cell->getGoalAircraft()==objID) continue; - return false; - } - if (cell->getType()==PathfindCell::CELL_OBSTACLE) { - if (cell->isObstaclePresent( ignoreId )) - continue; - return false; - } - if (IS_IMPASSABLE(cell->getType())) { - return false; + if (!cell) { + return false; // off the map, so can't place here. + } + + if (checkForAircraft) { + if (!cell->isAircraftGoal()) { + continue; } - if (cell->getFlags() == PathfindCell::NO_UNITS) { - continue; // Nobody is here, so it's ok. + if (cell->getGoalAircraft() == objID) { + continue; } - ObjectID goalUnitID = cell->getGoalUnit(); - if (goalUnitID==objID) { - continue; // we got it. - } else if (ignoreId==goalUnitID) { - continue; // we are ignoring it. - } else if (goalUnitID!=INVALID_ID) { - if (obj==NULL) { - return false; - } - Object *unit = TheGameLogic->findObjectByID(goalUnitID); - if (unit) { - // order matters: we want to know if I consider it to be an ally, not vice versa - if (obj->getRelationship(unit) == ALLIES) { - return false; // Don't usurp your allies goals. jba. - } - if (cell->getFlags()==PathfindCell::UNIT_PRESENT_FIXED) { - Bool canCrush = obj->canCrushOrSquish(unit, TEST_CRUSH_OR_SQUISH); - if (!canCrush) { - return false; // Don't move to an occupied cell. - } - } - } + return false; + } + + if (cell->getType()==PathfindCell::CELL_OBSTACLE) { + if (cell->isObstaclePresent( ignoreId )) + continue; + return false; + } + + if (IS_IMPASSABLE(cell->getType())) { + return false; + } + + if (cell->getFlags() == PathfindCell::NO_UNITS) { + continue; // Nobody is here, so it's ok. + } + + ObjectID goalUnitID = cell->getGoalUnit(); + if (goalUnitID == objID) { + continue; // we got it. + } + + if (goalUnitID == ignoreId) { + continue; // we are ignoring it. + } + + if (goalUnitID == INVALID_ID) { + continue; + } + + if (!obj) { + return false; + } + Object *unit = TheGameLogic->findObjectByID(goalUnitID); + if (!unit) { + continue; + } + + // order matters: we want to know if I consider it to be an ally, not vice versa + if (obj->getRelationship(unit) == ALLIES) { + return false; // Don't usurp your allies goals. jba. + } + if (cell->getFlags()==PathfindCell::UNIT_PRESENT_FIXED) { + Bool canCrush = obj->canCrushOrSquish(unit, TEST_CRUSH_OR_SQUISH); + if (!canCrush) { + return false; // Don't move to an occupied cell. } - } else { - return false; // off the map, so can't place here. } } } - return true; } From 82873b11d06998adef9e8adcccff3196f3ca8e30 Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Wed, 24 Sep 2025 19:11:33 +0100 Subject: [PATCH 2/5] refactor(pathfinder): Simplify and improve readability of Pathfinder::snapClosestGoalPosition (#1645) --- .../Source/GameLogic/AI/AIPathfind.cpp | 52 ++++++++++--------- .../Source/GameLogic/AI/AIPathfind.cpp | 52 ++++++++++--------- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index fc1d65aa81..c28fe7ed2a 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -4552,37 +4552,41 @@ void Pathfinder::snapClosestGoalPosition(Object *obj, Coord3D *pos) // Try adjusting by 1. Int i,j; - for (i=cell.x-1; igetGoalUnit()==INVALID_ID || newCell->getGoalUnit()==obj->getID()) { - adjustCoordToCell(i, j, center, *pos, layer); - return; - } - } + + if (iRadius > 0) + return; + + // Try to find an unoccupied cell. + for (i = cell.x - 1; i < cell.x + 2; i++) { + for (j = cell.y - 1; j < cell.y + 2; j++) { + PathfindCell* newCell = getCell(layer, i, j); + if (!newCell) + continue; + + if (newCell->getGoalUnit() == INVALID_ID || newCell->getGoalUnit() == obj->getID()) { + adjustCoordToCell(i, j, center, *pos, layer); + return; } } - // Try to find an unoccupied cell. - for (i=cell.x-1; igetFlags()!=PathfindCell::UNIT_PRESENT_FIXED) { - adjustCoordToCell(i, j, center, *pos, layer); - return; - } - } + } + + for (i = cell.x - 1; i < cell.x + 2; i++) { + for (j = cell.y - 1; j < cell.y + 2; j++) { + PathfindCell* newCell = getCell(layer, i, j); + if (!newCell) + continue; + + if (newCell->getFlags()!=PathfindCell::UNIT_PRESENT_FIXED) { + adjustCoordToCell(i, j, center, *pos, layer); + return; } } } diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index c4cc68738b..013acde594 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -4837,37 +4837,41 @@ void Pathfinder::snapClosestGoalPosition(Object *obj, Coord3D *pos) // Try adjusting by 1. Int i,j; - for (i=cell.x-1; igetGoalUnit()==INVALID_ID || newCell->getGoalUnit()==obj->getID()) { - adjustCoordToCell(i, j, center, *pos, layer); - return; - } - } + + if (iRadius > 0) + return; + + // Try to find an unoccupied cell. + for (i = cell.x - 1; i < cell.x + 2; i++) { + for (j = cell.y - 1; j < cell.y + 2; j++) { + PathfindCell* newCell = getCell(layer, i, j); + if (!newCell) + continue; + + if (newCell->getGoalUnit() == INVALID_ID || newCell->getGoalUnit() == obj->getID()) { + adjustCoordToCell(i, j, center, *pos, layer); + return; } } - // Try to find an unoccupied cell. - for (i=cell.x-1; igetFlags()!=PathfindCell::UNIT_PRESENT_FIXED) { - adjustCoordToCell(i, j, center, *pos, layer); - return; - } - } + } + + for (i = cell.x - 1; i < cell.x + 2; i++) { + for (j = cell.y - 1; j < cell.y + 2; j++) { + PathfindCell* newCell = getCell(layer, i, j); + if (!newCell) + continue; + + if (newCell->getFlags()!=PathfindCell::UNIT_PRESENT_FIXED) { + adjustCoordToCell(i, j, center, *pos, layer); + return; } } } From f428ec0b688fc9b0127dc118f427f8dd98c5f493 Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Sun, 28 Sep 2025 09:35:20 +0100 Subject: [PATCH 3/5] refactor(pathfinder): Simplify and improve readability of Pathfinder::examineCellsCallback (#1645) --- .../Source/GameLogic/AI/AIPathfind.cpp | 32 ++++++++----------- .../Source/GameLogic/AI/AIPathfind.cpp | 32 ++++++++----------- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index c28fe7ed2a..cec570612d 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -5413,14 +5413,7 @@ struct ExamineCellsStruct if ( (to->getLayer() == LAYER_GROUND) && !d->thePathfinder->m_zoneManager.isPassable(to_x, to_y) ) { return 1; } - Bool onList = false; - if (to->hasInfo()) { - if (to->getOpen() || to->getClosed()) - { - // already on one of the lists - onList = true; - } - } + if (to->getPinched()) { return 1; // abort. } @@ -5439,25 +5432,26 @@ struct ExamineCellsStruct info.radius = d->radius; info.considerTransient = false; info.acceptableSurfaces = d->theLoco->getValidSurfaces(); - if (!d->thePathfinder->checkForMovement(d->obj, info) || info.enemyFixed) { + if (!d->thePathfinder->checkForMovement(d->obj, info)) { return 1; //abort. } + if (info.enemyFixed) { return 1; //abort. } - ICoord2D newCellCoord; - newCellCoord.x = to_x; - newCellCoord.y = to_y; + + if (info.allyFixedCount) { + return 1; //abort. + } UnsignedInt newCostSoFar = from->getCostSoFar( ) + 0.5f*COST_ORTHOGONAL; if (to->getType() == PathfindCell::CELL_CLIFF ) { return 1; } - if (info.allyFixedCount) { - return 1; - } else if (info.enemyFixed) { - return 1; - } + + ICoord2D newCellCoord; + newCellCoord.x = to_x; + newCellCoord.y = to_y; if (!to->allocateInfo(newCellCoord)) { // Out of cells for pathing... @@ -5466,15 +5460,17 @@ struct ExamineCellsStruct to->setBlockedByAlly(false); Int costRemaining = 0; costRemaining = to->costToGoal( d->goalCell ); + // check if this neighbor cell is already on the open (waiting to be tried) // or closed (already tried) lists - if (onList) + if ( to->hasInfo() && (to->getOpen() || to->getClosed()) ) { // already on one of the lists - if existing costSoFar is less, // the new cell is on a longer path, so skip it if (to->getCostSoFar() <= newCostSoFar) return 0; // keep going. } + to->setCostSoFar(newCostSoFar); // keep track of path we're building - point back to cell we moved here from to->setParentCell(from) ; diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 013acde594..32a7315495 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -5712,14 +5712,7 @@ struct ExamineCellsStruct if ( (to->getLayer() == LAYER_GROUND) && !d->thePathfinder->m_zoneManager.isPassable(to_x, to_y) ) { return 1; } - Bool onList = false; - if (to->hasInfo()) { - if (to->getOpen() || to->getClosed()) - { - // already on one of the lists - onList = true; - } - } + if (to->getPinched()) { return 1; // abort. } @@ -5738,25 +5731,26 @@ struct ExamineCellsStruct info.radius = d->radius; info.considerTransient = false; info.acceptableSurfaces = d->theLoco->getValidSurfaces(); - if (!d->thePathfinder->checkForMovement(d->obj, info) || info.enemyFixed) { + if (!d->thePathfinder->checkForMovement(d->obj, info)) { return 1; //abort. } + if (info.enemyFixed) { return 1; //abort. } - ICoord2D newCellCoord; - newCellCoord.x = to_x; - newCellCoord.y = to_y; + + if (info.allyFixedCount) { + return 1; //abort. + } UnsignedInt newCostSoFar = from->getCostSoFar( ) + 0.5f*COST_ORTHOGONAL; if (to->getType() == PathfindCell::CELL_CLIFF ) { return 1; } - if (info.allyFixedCount) { - return 1; - } else if (info.enemyFixed) { - return 1; - } + + ICoord2D newCellCoord; + newCellCoord.x = to_x; + newCellCoord.y = to_y; if (!to->allocateInfo(newCellCoord)) { // Out of cells for pathing... @@ -5765,15 +5759,17 @@ struct ExamineCellsStruct to->setBlockedByAlly(false); Int costRemaining = 0; costRemaining = to->costToGoal( d->goalCell ); + // check if this neighbor cell is already on the open (waiting to be tried) // or closed (already tried) lists - if (onList) + if ( to->hasInfo() && (to->getOpen() || to->getClosed()) ) { // already on one of the lists - if existing costSoFar is less, // the new cell is on a longer path, so skip it if (to->getCostSoFar() <= newCostSoFar) return 0; // keep going. } + to->setCostSoFar(newCostSoFar); // keep track of path we're building - point back to cell we moved here from to->setParentCell(from) ; From 9df91d8abcd2abeb21862668198c8fe67efb3631 Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Sun, 28 Sep 2025 10:01:04 +0100 Subject: [PATCH 4/5] refactor(pathfinder): Simplify and improve readability of Pathfinder::checkForMovement (#1645) --- .../Source/GameLogic/AI/AIPathfind.cpp | 150 +++++++++--------- .../Source/GameLogic/AI/AIPathfind.cpp | 144 +++++++++-------- 2 files changed, 155 insertions(+), 139 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index cec570612d..6dfb149148 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -4415,7 +4415,10 @@ Bool Pathfinder::checkForMovement(const Object *obj, TCheckMovementInfo &info) ObjectID allies[maxAlly]; Int numAlly = 0; - if (!obj) return true; // not object can move there. + if (!obj) { + return true; // not object can move there. + } + ObjectID ignoreId = INVALID_ID; if (obj->getAIUpdateInterface()) { ignoreId = obj->getAIUpdateInterface()->getIgnoredObstacleID(); @@ -4428,81 +4431,86 @@ Bool Pathfinder::checkForMovement(const Object *obj, TCheckMovementInfo &info) for (i=info.cell.x-info.radius; igetFlags(); - ObjectID posUnit = cell->getPosUnit(); - if ((flags == PathfindCell::UNIT_GOAL) || (flags == PathfindCell::UNIT_GOAL_OTHER_MOVING)) { - info.allyGoal = true; - } - if (flags == PathfindCell::NO_UNITS) { - continue; // Nobody is here, so it's ok. - } else if (posUnit==obj->getID()) { - continue; // we got it. - } else if (posUnit==ignoreId) { - continue; // we are ignoring this one. - } else { - Bool check = false; - Object *unit = NULL; - if (flags==PathfindCell::UNIT_PRESENT_MOVING || flags==PathfindCell::UNIT_GOAL_OTHER_MOVING) { - unit = TheGameLogic->findObjectByID(posUnit); - // order matters: we want to know if I consider it to be an ally, not vice versa - if (unit && obj->getRelationship(unit) == ALLIES) { - info.allyMoving = true; - } - if (info.considerTransient) { - check = true; - } - } - if (flags == PathfindCell::UNIT_PRESENT_FIXED) { - check = true; - unit = TheGameLogic->findObjectByID(posUnit); - } - if (check && unit!=NULL) { - if (obj->getAIUpdateInterface() && obj->getAIUpdateInterface()->getIgnoredObstacleID()==unit->getID()) { - // Don't check if it's the ignored obstacle. - check = false; - } - } - if (check && unit) { + if (!cell) { + return false; // off the map, so can't move here. + } + + PathfindCell::CellFlags flags = cell->getFlags(); + if ((flags == PathfindCell::UNIT_GOAL) || (flags == PathfindCell::UNIT_GOAL_OTHER_MOVING)) { + info.allyGoal = true; + } else if (flags == PathfindCell::NO_UNITS) { + continue; // Nobody is here, so it's ok. + } + + ObjectID posUnit = cell->getPosUnit(); + if (posUnit == obj->getID()) { + continue; // we got it. + } + + if (posUnit == ignoreId) { + continue; // we are ignoring this one. + } + + Bool check = false; + Object *unit = NULL; + if (flags == PathfindCell::UNIT_PRESENT_MOVING || flags == PathfindCell::UNIT_GOAL_OTHER_MOVING) { + unit = TheGameLogic->findObjectByID(posUnit); + // order matters: we want to know if I consider it to be an ally, not vice versa + if (unit && obj->getRelationship(unit) == ALLIES) { + info.allyMoving = true; + } + if (info.considerTransient) { + check = true; + } + } + if (flags == PathfindCell::UNIT_PRESENT_FIXED) { + check = true; + unit = TheGameLogic->findObjectByID(posUnit); + } + if (check && unit!=NULL) { + if (obj->getAIUpdateInterface() && obj->getAIUpdateInterface()->getIgnoredObstacleID()==unit->getID()) { + // Don't check if it's the ignored obstacle. + check = false; + } + } + if (!check || !unit) { + continue; + } + #ifdef INFANTRY_MOVES_THROUGH_INFANTRY - if (obj->isKindOf(KINDOF_INFANTRY) && unit->isKindOf(KINDOF_INFANTRY)) { - // Infantry can run through infantry. - continue; // - } + if (obj->isKindOf(KINDOF_INFANTRY) && unit->isKindOf(KINDOF_INFANTRY)) { + // Infantry can run through infantry. + continue; // + } #endif - // See if it is an ally. - // order matters: we want to know if I consider it to be an ally, not vice versa - if (obj->getRelationship(unit) == ALLIES) { - if (!unit->getAIUpdateInterface()) { - return false; // can't path through not-idle units. - } - if (!unit->getAIUpdateInterface()->isIdle()) { - return false; // can't path through not-idle units. - } - Bool found = false; - Int k; - for (k=0; kgetID()) { - found = true; - } - } - if (!found) { - info.allyFixedCount++; - if (numAlly < maxAlly) { - allies[numAlly] = unit->getID(); - numAlly++; - } - } - } else { - Bool canCrush = obj->canCrushOrSquish( unit, TEST_CRUSH_OR_SQUISH ); - if (!canCrush) { - info.enemyFixed = true; - } - } + // See if it is an ally. + // order matters: we want to know if I consider it to be an ally, not vice versa + if (obj->getRelationship(unit) == ALLIES) { + if (!unit->getAIUpdateInterface()) { + return false; // can't path through not-idle units. + } + if (!unit->getAIUpdateInterface()->isIdle()) { + return false; // can't path through not-idle units. + } + Bool found = false; + Int k; + for (k=0; kgetID()) { + found = true; + } + } + if (!found) { + info.allyFixedCount++; + if (numAlly < maxAlly) { + allies[numAlly] = unit->getID(); + numAlly++; } } } else { - return false; // off the map, so can't place here. + Bool canCrush = obj->canCrushOrSquish( unit, TEST_CRUSH_OR_SQUISH ); + if (!canCrush) { + info.enemyFixed = true; + } } } } diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 32a7315495..10f8772c66 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -4703,7 +4703,10 @@ Bool Pathfinder::checkForMovement(const Object *obj, TCheckMovementInfo &info) ObjectID allies[maxAlly]; Int numAlly = 0; - if (!obj) return true; // not object can move there. + if (!obj) { + return true; // not object can move there. + } + ObjectID ignoreId = INVALID_ID; if (obj->getAIUpdateInterface()) { ignoreId = obj->getAIUpdateInterface()->getIgnoredObstacleID(); @@ -4716,78 +4719,83 @@ Bool Pathfinder::checkForMovement(const Object *obj, TCheckMovementInfo &info) for (i=info.cell.x-info.radius; igetFlags(); - ObjectID posUnit = cell->getPosUnit(); - if ((flags == PathfindCell::UNIT_GOAL) || (flags == PathfindCell::UNIT_GOAL_OTHER_MOVING)) { - info.allyGoal = true; - } - if (flags == PathfindCell::NO_UNITS) { - continue; // Nobody is here, so it's ok. - } else if (posUnit==obj->getID()) { - continue; // we got it. - } else if (posUnit==ignoreId) { - continue; // we are ignoring this one. - } else { - Bool check = false; - Object *unit = NULL; - if (flags==PathfindCell::UNIT_PRESENT_MOVING || flags==PathfindCell::UNIT_GOAL_OTHER_MOVING) { - unit = TheGameLogic->findObjectByID(posUnit); - // order matters: we want to know if I consider it to be an ally, not vice versa - if (unit && obj->getRelationship(unit) == ALLIES) { - info.allyMoving = true; - } - if (info.considerTransient) { - check = true; - } - } - if (flags == PathfindCell::UNIT_PRESENT_FIXED) { - check = true; - unit = TheGameLogic->findObjectByID(posUnit); - } - if (check && unit!=NULL) { - if (obj->getAIUpdateInterface() && obj->getAIUpdateInterface()->getIgnoredObstacleID()==unit->getID()) { - // Don't check if it's the ignored obstacle. - check = false; - } - } - if (check && unit) { + if (!cell) { + return false; // off the map, so can't move here. + } + + PathfindCell::CellFlags flags = cell->getFlags(); + if ((flags == PathfindCell::UNIT_GOAL) || (flags == PathfindCell::UNIT_GOAL_OTHER_MOVING)) { + info.allyGoal = true; + } else if (flags == PathfindCell::NO_UNITS) { + continue; // Nobody is here, so it's ok. + } + + ObjectID posUnit = cell->getPosUnit(); + if (posUnit == obj->getID()) { + continue; // we got it. + } + + if (posUnit == ignoreId) { + continue; // we are ignoring this one. + } + + Bool check = false; + Object *unit = NULL; + if (flags == PathfindCell::UNIT_PRESENT_MOVING || flags == PathfindCell::UNIT_GOAL_OTHER_MOVING) { + unit = TheGameLogic->findObjectByID(posUnit); + // order matters: we want to know if I consider it to be an ally, not vice versa + if (unit && obj->getRelationship(unit) == ALLIES) { + info.allyMoving = true; + } + if (info.considerTransient) { + check = true; + } + } + if (flags == PathfindCell::UNIT_PRESENT_FIXED) { + check = true; + unit = TheGameLogic->findObjectByID(posUnit); + } + if (check && unit!=NULL) { + if (obj->getAIUpdateInterface() && obj->getAIUpdateInterface()->getIgnoredObstacleID()==unit->getID()) { + // Don't check if it's the ignored obstacle. + check = false; + } + } + if (!check || !unit) { + continue; + } + #ifdef INFANTRY_MOVES_THROUGH_INFANTRY - if (obj->isKindOf(KINDOF_INFANTRY) && unit->isKindOf(KINDOF_INFANTRY)) { - // Infantry can run through infantry. - continue; // - } + if (obj->isKindOf(KINDOF_INFANTRY) && unit->isKindOf(KINDOF_INFANTRY)) { + // Infantry can run through infantry. + continue; // + } #endif - // See if it is an ally. - // order matters: we want to know if I consider it to be an ally, not vice versa - if (obj->getRelationship(unit) == ALLIES) { - if (!unit->getAIUpdateInterface()) { - return false; // can't path through not-idle units. - } - Bool found = false; - Int k; - for (k=0; kgetID()) { - found = true; - } - } - if (!found) { - info.allyFixedCount++; - if (numAlly < maxAlly) { - allies[numAlly] = unit->getID(); - numAlly++; - } - } - } else { - Bool canCrush = obj->canCrushOrSquish( unit, TEST_CRUSH_OR_SQUISH ); - if (!canCrush) { - info.enemyFixed = true; - } - } + // See if it is an ally. + // order matters: we want to know if I consider it to be an ally, not vice versa + if (obj->getRelationship(unit) == ALLIES) { + if (!unit->getAIUpdateInterface()) { + return false; // can't path through not-idle units. + } + Bool found = false; + Int k; + for (k=0; kgetID()) { + found = true; + } + } + if (!found) { + info.allyFixedCount++; + if (numAlly < maxAlly) { + allies[numAlly] = unit->getID(); + numAlly++; } } } else { - return false; // off the map, so can't place here. + Bool canCrush = obj->canCrushOrSquish( unit, TEST_CRUSH_OR_SQUISH ); + if (!canCrush) { + info.enemyFixed = true; + } } } } From fa7bc601ae9e7e0dca388f95a6a0ac784238ddcc Mon Sep 17 00:00:00 2001 From: Mauller <26652186+Mauller@users.noreply.github.com> Date: Tue, 30 Sep 2025 21:51:28 +0100 Subject: [PATCH 5/5] refactor(pathfinder): Simplify and improve readability of Pathfinder::moveAllies (#1645) --- .../Source/GameLogic/AI/AIPathfind.cpp | 71 +++++++++------ .../Source/GameLogic/AI/AIPathfind.cpp | 90 +++++++++++-------- 2 files changed, 98 insertions(+), 63 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 6dfb149148..bbe5e29e69 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -9555,10 +9555,14 @@ if (g_UT_startTiming) return false; #endif if (!obj->isKindOf(KINDOF_DOZER) && !obj->isKindOf(KINDOF_HARVESTER)) { // Harvesters & dozers want a clear path. - if (!path->getBlockedByAlly()) return FALSE; // Only move units if it is required. + if (!path->getBlockedByAlly()) { + return FALSE; // Only move units if it is required. + } } LatchRestore recursiveDepth(m_moveAlliesDepth, m_moveAlliesDepth+1); - if (m_moveAlliesDepth > 2) return false; + if (m_moveAlliesDepth > 2) { + return false; + } Bool centerInCell; Int radius; @@ -9577,33 +9581,48 @@ if (g_UT_startTiming) return false; for (i=curCell.x-radius; igetLayer(), i, j); - if (cell) { - if (cell->getPosUnit()==INVALID_ID) { + if (!cell) { + continue; // Cell is not on the pathfinding grid + } + + ObjectID unitId = cell->getPosUnit(); + if (unitId==INVALID_ID) { + continue; + } + + if (unitId==obj->getID()) { + continue; // It's us. + } + + if (unitId==ignoreId) { + continue; // It's the one we are ignoring. + } + + Object *otherObj = TheGameLogic->findObjectByID(unitId); + if (!otherObj) { + continue; + } + + if (obj->getRelationship(otherObj)!=ALLIES) { + continue; // Only move allies. + } + + if (obj->isKindOf(KINDOF_INFANTRY) && otherObj->isKindOf(KINDOF_INFANTRY)) { + continue; // infantry can walk through other infantry, so just let them. + } + if (obj->isKindOf(KINDOF_INFANTRY) && !otherObj->isKindOf(KINDOF_INFANTRY)) { + // If this is a general clear operation, don't let infantry push vehicles. + if (!path->getBlockedByAlly()) { continue; } - if (cell->getPosUnit()==obj->getID()) { - continue; // It's us. - } - if (cell->getPosUnit()==ignoreId) { - continue; // It's the one we are ignoring. - } - Object *otherObj = TheGameLogic->findObjectByID(cell->getPosUnit()); - if (obj->getRelationship(otherObj)!=ALLIES) { - continue; // Only move allies. - } - if (otherObj==NULL) continue; - if (obj->isKindOf(KINDOF_INFANTRY) && otherObj->isKindOf(KINDOF_INFANTRY)) { - continue; // infantry can walk through other infantry, so just let them. - } - if (obj->isKindOf(KINDOF_INFANTRY) && !otherObj->isKindOf(KINDOF_INFANTRY)) { - // If this is a general clear operation, don't let infantry push vehicles. - if (!path->getBlockedByAlly()) continue; - } - if (otherObj && otherObj->getAI() && !otherObj->getAI()->isMoving()) { - //DEBUG_LOG(("Moving ally")); - otherObj->getAI()->aiMoveAwayFromUnit(obj, CMD_FROM_AI); - } } + + if (!otherObj->getAI() || otherObj->getAI()->isMoving()) { + continue; + } + + //DEBUG_LOG(("Moving ally")); + otherObj->getAI()->aiMoveAwayFromUnit(obj, CMD_FROM_AI); } } } diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp index 10f8772c66..43b135c80f 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp @@ -9967,10 +9967,14 @@ if (g_UT_startTiming) return false; #endif if (!obj->isKindOf(KINDOF_DOZER) && !obj->isKindOf(KINDOF_HARVESTER)) { // Harvesters & dozers want a clear path. - if (!path->getBlockedByAlly()) return FALSE; // Only move units if it is required. + if (!path->getBlockedByAlly()) { + return FALSE; // Only move units if it is required. + } } LatchRestore recursiveDepth(m_moveAlliesDepth, m_moveAlliesDepth+1); - if (m_moveAlliesDepth > 2) return false; + if (m_moveAlliesDepth > 2) { + return false; + } Bool centerInCell; Int radius; @@ -9989,46 +9993,58 @@ if (g_UT_startTiming) return false; for (i=curCell.x-radius; igetLayer(), i, j); - if (cell) { - if (cell->getPosUnit()==INVALID_ID) { + if (!cell) { + continue; // Cell is not on the pathfinding grid + } + + ObjectID unitId = cell->getPosUnit(); + if (unitId==INVALID_ID) { + continue; + } + + if (unitId==obj->getID()) { + continue; // It's us. + } + + if (unitId==ignoreId) { + continue; // It's the one we are ignoring. + } + + Object *otherObj = TheGameLogic->findObjectByID(unitId); + if (!otherObj) { + continue; + } + + if (obj->getRelationship(otherObj)!=ALLIES) { + continue; // Only move allies. + } + + if (obj->isKindOf(KINDOF_INFANTRY) && otherObj->isKindOf(KINDOF_INFANTRY)) { + continue; // infantry can walk through other infantry, so just let them. + } + if (obj->isKindOf(KINDOF_INFANTRY) && !otherObj->isKindOf(KINDOF_INFANTRY)) { + // If this is a general clear operation, don't let infantry push vehicles. + if (!path->getBlockedByAlly()) { continue; } - if (cell->getPosUnit()==obj->getID()) { - continue; // It's us. - } - if (cell->getPosUnit()==ignoreId) { - continue; // It's the one we are ignoring. - } - Object *otherObj = TheGameLogic->findObjectByID(cell->getPosUnit()); - if (obj->getRelationship(otherObj)!=ALLIES) { - continue; // Only move allies. - } - if (otherObj==NULL) continue; - if (obj->isKindOf(KINDOF_INFANTRY) && otherObj->isKindOf(KINDOF_INFANTRY)) { - continue; // infantry can walk through other infantry, so just let them. - } - if (obj->isKindOf(KINDOF_INFANTRY) && !otherObj->isKindOf(KINDOF_INFANTRY)) { - // If this is a general clear operation, don't let infantry push vehicles. - if (!path->getBlockedByAlly()) continue; - } - if( otherObj && otherObj->getAI() && !otherObj->getAI()->isMoving() ) - { - if( otherObj->getAI()->isAttacking() ) - { - continue; // Don't move units that are attacking. [8/14/2003] - } + } - //Kris: Patch 1.01 November 3, 2003 - //Black Lotus exploit fix -- moving while hacking. - if( otherObj->testStatus( OBJECT_STATUS_IS_USING_ABILITY ) || otherObj->getAI()->isBusy() ) - { - continue; // Packing or unpacking objects for example - } + if (!otherObj->getAI() || otherObj->getAI()->isMoving()) { + continue; + } - //DEBUG_LOG(("Moving ally")); - otherObj->getAI()->aiMoveAwayFromUnit(obj, CMD_FROM_AI); - } + if (otherObj->getAI()->isAttacking()) { + continue; // Don't move units that are attacking. [8/14/2003] + } + + //Kris: Patch 1.01 November 3, 2003 + //Black Lotus exploit fix -- moving while hacking. + if( otherObj->testStatus( OBJECT_STATUS_IS_USING_ABILITY ) || otherObj->getAI()->isBusy() ) { + continue; // Packing or unpacking objects for example } + + //DEBUG_LOG(("Moving ally")); + otherObj->getAI()->aiMoveAwayFromUnit(obj, CMD_FROM_AI); } } }