cursor shows "pick object" instead of "attack" about vcmi

[kambala-decapitator]

seems related to monolith indeed, one more example:

don't have save file, unfortunately

[kambala-decapitator]

found an easy way to reproduce and not related to monoliths: just start RoE - Long Live the Queen campaign.

with 2 creatures also reproducible:

when a single creature is left, cursor becomes correct:

[kambala-decapitator]

one more way to reproduce: start AB campaign.

[kambala-decapitator]

seems the bug is here:

vcmi/client/windows/CAdvmapInterface.cpp

Line 1715 in e423e41

const CGPathNode * pnode = LOCPLINT->cb->getPathsInfo(h)->getPathInfo(mapPosCopy);

pnode->action is wrong for creature tiles.

[kambala-decapitator]

seems I found where it got broken: 49c872e

the following simple patch fixes the issue, but I'm not sure about its consequences:

diff --git a/lib/CPathfinder.cpp b/lib/CPathfinder.cpp
index 6ae919db0..33f90bb39 100644
--- a/lib/CPathfinder.cpp
+++ b/lib/CPathfinder.cpp
@@ -862,7 +862,7 @@ void DestinationActionRule::process(
 				else
 					action = CGPathNode::BLOCKING_VISIT;
 			}
-			else if(destination.isGuardianTile)
+			else if(destination.guarded)
 				action = CGPathNode::BATTLE;
 			else if(destination.nodeObject->blockVisit && !(pathfinderConfig->options.useCastleGate && destination.nodeObject->ID == Obj::TOWN))
 				action = CGPathNode::BLOCKING_VISIT;

@nullkiller could you double-check since that's your commit?

branch with the change: https://github.com/kambala-decapitator/vcmi/tree/fix-cursor-attack

[kambala-decapitator]

it seems that the right fix is actually:

diff --git a/lib/CPathfinder.cpp b/lib/CPathfinder.cpp
index 6ae919db0..ceab0c5e4 100644
--- a/lib/CPathfinder.cpp
+++ b/lib/CPathfinder.cpp
@@ -392,7 +392,7 @@ void CPathfinder::calculatePaths()
 			destination.movementLeft = movement;
 			destination.cost = cost;
 			destination.updateInfo(hlp, gs);
-			destination.isGuardianTile = destination.guarded && isDestinationGuardian();
+			destination.isGuardianTile = destination.guarded || isDestinationGuardian();
 
 			for(auto rule : config->rules)
 			{

[kambala-decapitator]

the latter is actually wrong: such stuff becomes possible which differs from the original (although battle begins as well when you try to acquire the sawmill)

with the former change original behavior is preserved:

[kambala-decapitator]

unguarded resources can still be captured with the former change: