Skip to content

Commit

Permalink
Fix the regression and minor issue in the AI pathfinder, fix crash in…
Browse files Browse the repository at this point in the history 
… the priority task management due to an assertion failure, improve the distance estimation when an AI-controlled hero passes through objects on the map (#7567)
  • Loading branch information
@oleg-derevenetz
oleg-derevenetz authored Aug 20, 2023
1 parent ccede12 commit 72df201
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 47 deletions.
5 changes: 4 additions & 1 deletion src/fheroes2/ai/ai_hero_action.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1146,9 +1146,12 @@ namespace
{
DEBUG_LOG( DBG_AI, DBG_INFO, hero.GetName() << ", object: " << MP2::StringObject( objectType ) )

assert( hero.GetIndex() == dst_index );
assert( hero.GetIndex() == dst_index || MP2::isNeedStayFront( objectType ) );

if ( !AI::Get().isValidHeroObject( hero, dst_index, ( hero.GetIndex() == dst_index ) ) ) {
// If we can't step on this tile, then we shouldn't be here at all
assert( !MP2::isNeedStayFront( objectType ) );

// We're just passing through here, don't mess with this object
DEBUG_LOG( DBG_AI, DBG_INFO, hero.GetName() << " passes through without interacting with the object" )
return;
Expand Down
11 changes: 7 additions & 4 deletions src/fheroes2/ai/normal/ai_normal_hero.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2186,11 +2186,14 @@ namespace AI
case PriorityTaskType::DEFEND:
case PriorityTaskType::REINFORCE: {
if ( hero.GetIndex() != tileIndex ) {
// The castle has just been captured. No task should be updated.
// Either the castle has just been captured, or the hero meets the guest hero of a friendly castle. No task should be updated.
// If any of these assertions blow up, then this is not one of these cases.
#ifndef NDEBUG
const Maps::Tiles & tile = world.GetTiles( tileIndex );
#endif
assert( tile.GetObject( false ) == MP2::OBJ_CASTLE && hero.GetColor() == Maps::getColorFromTile( tile ) );
assert( Maps::isValidDirection( tileIndex, Direction::BOTTOM ) && hero.GetIndex() == Maps::GetDirectionIndex( tileIndex, Direction::BOTTOM ) );

// If this assertion blows up then it is not the case described above.
assert( ( world.GetTiles( tileIndex ).GetObject() == MP2::OBJ_CASTLE ) && Maps::isValidDirection( tileIndex, Direction::BOTTOM )
&& hero.GetIndex() == Maps::GetDirectionIndex( tileIndex, Direction::BOTTOM ) );
return;
}

Expand Down
117 changes: 84 additions & 33 deletions src/fheroes2/world/world_pathfinding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ namespace
return ( art.GetID() == conf.WinsFindArtifactID() );
}

bool isTileAvailableForWalkThrough( int tileIndex, bool fromWater )
bool isTileAvailableForWalkThrough( const int tileIndex, const bool fromWater )
{
const Maps::Tiles & tile = world.GetTiles( tileIndex );
const bool toWater = tile.isWater();
Expand All @@ -98,12 +98,13 @@ namespace
return true;
}

bool isTileAvailableForWalkThroughForAIWithArmy( const int tileIndex, const int color, const bool isArtifactsBagFull, const double armyStrength,
bool isTileAvailableForWalkThroughForAIWithArmy( const int tileIndex, const bool fromWater, const int color, const bool isArtifactsBagFull, const double armyStrength,
const double minimalAdvantage )
{
assert( color & Color::ALL );

const Maps::Tiles & tile = world.GetTiles( tileIndex );
const bool toWater = tile.isWater();
const MP2::MapObjectType objectType = tile.GetObject();

const auto isTileAccessible = [color, armyStrength, minimalAdvantage, &tile]() {
Expand All @@ -122,6 +123,13 @@ namespace

// Enemy heroes can be defeated and passed through
if ( objectType == MP2::OBJ_HEROES ) {
// Heroes on the water can be attacked from the nearby shore, but they cannot be passed through
if ( fromWater != toWater ) {
assert( !fromWater && toWater );

return false;
}

const Heroes * otherHero = tile.GetHeroes();
assert( otherHero != nullptr );

Expand Down Expand Up @@ -217,9 +225,10 @@ namespace
return false;
}

// Although we can step on a castle's tile, there's nowhere to go through it anyway
// The castle tile can be passed through if we got there using Town Gate or Town Portal spells, which means that
// this should be our castle
if ( objectType == MP2::OBJ_CASTLE ) {
return false;
return color == Maps::getColorFromTile( tile );
}

// If we can step on this tile, but it is protected by monsters and it is impossible to refuse a fight, then it
Expand All @@ -232,7 +241,7 @@ namespace
return true;
}

bool isMovementAllowedForColor( const int from, const int direction, const int heroColor, const bool isSummonBoatSpellAvailable )
bool isMovementAllowedForColor( const int from, const int direction, const int color, const bool isSummonBoatSpellAvailable )
{
const Maps::Tiles & fromTile = world.GetTiles( from );
const bool fromWater = fromTile.isWater();
Expand Down Expand Up @@ -288,7 +297,7 @@ namespace

const Maps::Tiles & toTile = world.GetTiles( Maps::GetDirectionIndex( from, direction ) );

if ( toTile.isPassableFrom( Direction::Reflect( direction ), fromWater, false, heroColor ) ) {
if ( toTile.isPassableFrom( Direction::Reflect( direction ), fromWater, false, color ) ) {
return true;
}

Expand All @@ -303,7 +312,7 @@ namespace
}

// ... and this tile should be reachable from the shore (as if this shore tile were a water tile)
return toTile.isPassableFrom( Direction::Reflect( direction ), true, false, heroColor );
return toTile.isPassableFrom( Direction::Reflect( direction ), true, false, color );
}

bool isTileAccessibleForAIWithArmy( const int tileIndex, const double armyStrength, const double minimalAdvantage )
Expand Down Expand Up @@ -528,16 +537,17 @@ std::list<Route::Step> PlayerWorldPathfinder::buildPath( const int targetIndex )

void PlayerWorldPathfinder::processCurrentNode( std::vector<int> & nodesToExplore, const int currentNodeIdx )
{
const bool isFirstNode = currentNodeIdx == _pathStart;
const bool isFirstNode = ( currentNodeIdx == _pathStart );
const WorldNode & currentNode = _cache[currentNodeIdx];
const bool fromWater = world.GetTiles( _pathStart ).isWater();

if ( !isFirstNode && !isTileAvailableForWalkThrough( currentNodeIdx, world.GetTiles( _pathStart ).isWater() ) ) {
if ( !isFirstNode && !isTileAvailableForWalkThrough( currentNodeIdx, fromWater ) ) {
return;
}

const MapsIndexes & monsters = Maps::getMonstersProtectingTile( currentNodeIdx );

// If the current tile is protected, then the hero can only move to one of the neighboring monsters
// If the current tile is protected by monsters, and this tile is not the starting tile, then the hero can only move towards one of the neighboring monsters
if ( !isFirstNode && !monsters.empty() ) {
for ( int monsterIndex : monsters ) {
const int direction = Maps::GetDirection( currentNodeIdx, monsterIndex );
Expand Down Expand Up @@ -683,10 +693,10 @@ void AIWorldPathfinder::processWorldMap()
assert( castleIndex >= 0 && static_cast<size_t>( castleIndex ) < _cache.size() );
assert( castleIndex != _pathStart && _cache[castleIndex]._from == -1 );

const uint32_t movePointCost = spell.movePoints();
const uint32_t movePointsAfter = ( _remainingMovePoints < movePointCost ) ? 0 : _remainingMovePoints - movePointCost;
const uint32_t cost = spell.movePoints();
const uint32_t remaining = ( _remainingMovePoints < cost ) ? 0 : _remainingMovePoints - cost;

_cache[castleIndex] = WorldNode( _pathStart, movePointCost, MP2::OBJ_CASTLE, movePointsAfter );
_cache[castleIndex] = WorldNode( _pathStart, cost, MP2::OBJ_CASTLE, remaining );
nodesToExplore.push_back( castleIndex );
};

Expand Down Expand Up @@ -714,21 +724,26 @@ bool AIWorldPathfinder::isMovementAllowed( const int from, const int direction )

void AIWorldPathfinder::processCurrentNode( std::vector<int> & nodesToExplore, const int currentNodeIdx )
{
const bool isFirstNode = currentNodeIdx == _pathStart;
const bool isFirstNode = ( currentNodeIdx == _pathStart );
WorldNode & currentNode = _cache[currentNodeIdx];

const bool isAccessible = isTileAccessibleForAIWithArmy( currentNodeIdx, _armyStrength, _minimalArmyStrengthAdvantage );
// Always allow movement from the starting point to cover the edge case where we got here before this tile became blocked
if ( !isFirstNode ) {
if ( !isTileAccessibleForAIWithArmy( currentNodeIdx, _armyStrength, _minimalArmyStrengthAdvantage ) ) {
// If we can't move here, then reset the node
currentNode.resetNode();

// If we can't move here, reset
if ( !isAccessible ) {
currentNode.resetNode();
}
return;
}

// Always allow move from the starting spot to cover edge case if got there before tile became blocked/protected
if ( !isFirstNode
&& ( !isAccessible
|| !isTileAvailableForWalkThroughForAIWithArmy( currentNodeIdx, _color, _isArtifactsBagFull, _armyStrength, _minimalArmyStrengthAdvantage ) ) ) {
return;
// No dead ends allowed
assert( currentNode._from != -1 );

const bool fromWater = world.GetTiles( currentNode._from ).isWater();

if ( !isTileAvailableForWalkThroughForAIWithArmy( currentNodeIdx, fromWater, _color, _isArtifactsBagFull, _armyStrength, _minimalArmyStrengthAdvantage ) ) {
return;
}
}

MapsIndexes teleports;
Expand All @@ -742,20 +757,21 @@ void AIWorldPathfinder::processCurrentNode( std::vector<int> & nodesToExplore, c
}
}

// Do not check adjacent if we're going through the teleport in the middle of the path
// Check adjacent nodes only if we are either not on the teleport tile, or we got here from another endpoint of this teleport.
// Do not check them if we came to the tile with a teleport from a neighboring tile (and are going to use it for teleportation).
if ( teleports.empty() || std::find( teleports.begin(), teleports.end(), currentNode._from ) != teleports.end() ) {
checkAdjacentNodes( nodesToExplore, currentNodeIdx );
}

// Special case: move through teleports
// Special case: movement via teleport
for ( const int teleportIdx : teleports ) {
if ( teleportIdx == _pathStart ) {
continue;
}

WorldNode & teleportNode = _cache[teleportIdx];

// Check if move is actually faster through teleport
// Check if the movement is really faster via teleport
if ( teleportNode._from == -1 || teleportNode._cost > currentNode._cost ) {
const Maps::Tiles & teleportTile = world.GetTiles( teleportIdx );

Expand All @@ -771,10 +787,43 @@ void AIWorldPathfinder::processCurrentNode( std::vector<int> & nodesToExplore, c

uint32_t AIWorldPathfinder::getMovementPenalty( const int from, const int to, const int direction ) const
{
const uint32_t defaultPenalty = WorldPathfinder::getMovementPenalty( from, to, direction );
const uint32_t defaultPenalty = [this, from, to, direction]() {
const uint32_t regularPenalty = WorldPathfinder::getMovementPenalty( from, to, direction );

if ( from == _pathStart ) {
return regularPenalty;
}

const MP2::MapObjectType objectType = world.GetTiles( from ).GetObject();
if ( !MP2::isNeedStayFront( objectType ) || objectType == MP2::OBJ_BOAT ) {
return regularPenalty;
}

const WorldNode & node = _cache[from];

// If we perform pathfinding for a real AI-controlled hero on the map, we should encourage him
// to overcome water obstacles using boats.
// No dead ends allowed
assert( node._from != -1 );

const int prevStepDirection = Maps::GetDirection( node._from, from );
assert( prevStepDirection != Direction::UNKNOWN && prevStepDirection != Direction::CENTER );

// If we are moving from a tile that we technically cannot stand on, then it means that there was
// an object on this tile that we previously removed. Thus, we have spent additional movement points
// when moving to this tile - once when accessing the object to remove it, and again when moving to
// this tile.
//
// According to a rough estimate, the movement points spent can be considered the same in both cases,
// therefore, we apply an additional penalty when moving from the tile containing this object to the
// next tile. In general, it is impossible to perform an accurate estimation, since the stats and
// skills of a moving hero may change after interacting with the object.
//
// The real path will not reach this step, so this logic will be used to estimate distances more
// accurately when choosing whether to move through objects or past them.
return regularPenalty + WorldPathfinder::getMovementPenalty( node._from, from, prevStepDirection );
}();

// If we perform pathfinding for a real AI-controlled hero on the map, we should correctly calculate
// movement penalties when this hero overcomes water obstacles using boats.
if ( _maxMovePoints > 0 ) {
const WorldNode & node = _cache[from];

Expand All @@ -794,6 +843,8 @@ uint32_t AIWorldPathfinder::getMovementPenalty( const int from, const int to, co
// If the hero is not able to make this movement this turn, then he will have to spend
// all the movement points next turn.
if ( defaultPenalty > node._remainingMovePoints ) {
assert( _maxMovePoints >= defaultPenalty );

return _maxMovePoints;
}

Expand Down Expand Up @@ -1287,7 +1338,7 @@ std::list<Route::Step> AIWorldPathfinder::getDimensionDoorPath( const Heroes & h
return {};
}

std::list<Route::Step> AIWorldPathfinder::buildPath( const int targetIndex, const bool isPlanningMode /* = false */ ) const
std::list<Route::Step> AIWorldPathfinder::buildPath( const int targetIndex ) const
{
assert( _pathStart != -1 && targetIndex != -1 );

Expand Down Expand Up @@ -1328,8 +1379,8 @@ std::list<Route::Step> AIWorldPathfinder::buildPath( const int targetIndex, cons
currentNode = node._from;
}

// Cut the path to the last valid tile/obstacle if not in planning mode.
if ( !isPlanningMode && lastValidNode != targetIndex ) {
// Cut the path to the last valid tile/obstacle
if ( lastValidNode != targetIndex ) {
path.erase( std::find_if( path.begin(), path.end(), [lastValidNode]( const Route::Step & step ) { return step.GetFrom() == lastValidNode; } ), path.end() );
}

Expand Down
23 changes: 14 additions & 9 deletions src/fheroes2/world/world_pathfinding.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ class PlayerWorldPathfinder final : public WorldPathfinder
void reset() override;

void reEvaluateIfNeeded( const Heroes & hero );

// Builds and returns a path to the tile with the index 'targetIndex'. If the destination tile is not reachable,
// then an empty path is returned.
std::list<Route::Step> buildPath( const int targetIndex ) const;

private:
Expand All @@ -150,6 +153,7 @@ class AIWorldPathfinder final : public WorldPathfinder

void reEvaluateIfNeeded( const Heroes & hero );
void reEvaluateIfNeeded( const int start, const int color, const double armyStrength, const uint8_t skill );

int getFogDiscoveryTile( const Heroes & hero, bool & isTerritoryExpansion );

// Used for cases when heroes are stuck because one hero might be blocking the way and we have to move him.
Expand All @@ -161,13 +165,14 @@ class AIWorldPathfinder final : public WorldPathfinder

std::list<Route::Step> getDimensionDoorPath( const Heroes & hero, int targetIndex ) const;

// Builds and returns a path to the tile with the index 'targetIndex'. If there is a need to pass through any objects
// on the way to this tile, then a path to the nearest such object is returned. If the destination tile is not reachable
// in principle, then an empty path is returned.
std::list<Route::Step> buildPath( const int targetIndex ) const;

// Used for non-hero armies, like castles or monsters
uint32_t getDistance( int start, int targetIndex, int color, double armyStrength, uint8_t skill = Skill::Level::EXPERT );

// Override builds path to the nearest valid object
std::list<Route::Step> buildPath( const int targetIndex, const bool isPlanningMode = false ) const;

// Faster, but does not re-evaluate the map (expose base class method)
// Faster, but does not re-evaluate the map (exposed method of the base class)
using Pathfinder::getDistance;

// Returns the coefficient of the minimum required advantage in army strength in order to be able to "pass through"
Expand Down Expand Up @@ -201,10 +206,10 @@ class AIWorldPathfinder final : public WorldPathfinder
// Follows custom passability rules (for the AI)
void processCurrentNode( std::vector<int> & nodesToExplore, const int currentNodeIdx ) override;

// Adds special logic for AI-controlled heroes to encourage them to overcome water obstacles using boats. If this
// logic should be taken into account (when performing pathfinding for a real hero on the map), then the source
// tile should be already accessible for this hero and it should also have a valid information about the hero's
// remaining movement points.
// Adds special logic for AI-controlled heroes to correctly calculate movement penalties when such a hero passes
// through objects on the map or overcomes water obstacles using boats. If this logic should be taken into account
// (when performing pathfinding for a real hero on the map), then the source tile should be already accessible for
// this hero and it should also have a valid information about the hero's remaining movement points.
uint32_t getMovementPenalty( const int from, const int to, const int direction ) const override;

// Hero properties should be cached here because they can change even if the hero's position does not change,
Expand Down

0 comments on commit 72df201

Please sign in to comment.