Fixed most common cases of movement actions

AI/EmptyAI/CEmptyAI.cpp

@@ -61,7 +61,7 @@ void CEmptyAI::showBlockingDialog(const std::string &text, const std::vector<Com
 	cb->selectionMade(0, askID);
 }
 
-void CEmptyAI::showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
+void CEmptyAI::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
 {
 	cb->selectionMade(0, askID);
 }

AI/EmptyAI/CEmptyAI.h

@@ -29,7 +29,7 @@ public:
 	void heroGotLevel(const CGHeroInstance *hero, PrimarySkill pskill, std::vector<SecondarySkill> &skills, QueryID queryID) override;
 	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, QueryID queryID) override;
 	void showBlockingDialog(const std::string &text, const std::vector<Component> &components, QueryID askID, const int soundID, bool selection, bool cancel) override;
-	void showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
+	void showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
 	void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, QueryID queryID) override;
 	void showMapObjectSelectDialog(QueryID askID, const Component & icon, const MetaString & title, const MetaString & description, const std::vector<ObjectInstanceID> & objects) override;
 	std::optional<BattleAction> makeSurrenderRetreatDecision(const BattleID & battleID, const BattleStateInfoForRetreat & battleState) override;

AI/Nullkiller/AIGateway.cpp

@@ -654,7 +654,7 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 	});
 }
 
-void AIGateway::showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
+void AIGateway::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
 {
 	NET_EVENT_HANDLER;
 	status.addQuery(askID, boost::str(boost::format("Teleport dialog query with %d exits") % exits.size()));

AI/Nullkiller/AIGateway.h

@@ -117,7 +117,7 @@ public:
 	void commanderGotLevel(const CCommanderInstance * commander, std::vector<ui32> skills, QueryID queryID) override; //TODO
 	void showBlockingDialog(const std::string & text, const std::vector<Component> & components, QueryID askID, const int soundID, bool selection, bool cancel) override; //Show a dialog, player must take decision. If selection then he has to choose between one of given components, if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called with number of selected component (1 - n) or 0 for cancel (if allowed) and askID.
 	void showGarrisonDialog(const CArmedInstance * up, const CGHeroInstance * down, bool removableUnits, QueryID queryID) override; //all stacks operations between these objects become allowed, interface has to call onEnd when done
-	void showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
+	void showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
 	void showMapObjectSelectDialog(QueryID askID, const Component & icon, const MetaString & title, const MetaString & description, const std::vector<ObjectInstanceID> & objects) override;
 	void saveGame(BinarySerializer & h, const int version) override; //saving
 	void loadGame(BinaryDeserializer & h, const int version) override; //loading

AI/VCAI/VCAI.cpp

@@ -656,7 +656,7 @@ void VCAI::showBlockingDialog(const std::string & text, const std::vector<Compon
 	});
 }
 
-void VCAI::showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
+void VCAI::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
 {
 //	LOG_TRACE_PARAMS(logAi, "askID '%i', exits '%s'", askID % exits);
 	NET_EVENT_HANDLER;

AI/VCAI/VCAI.h

@@ -150,7 +150,7 @@ public:
 	void commanderGotLevel(const CCommanderInstance * commander, std::vector<ui32> skills, QueryID queryID) override; //TODO
 	void showBlockingDialog(const std::string & text, const std::vector<Component> & components, QueryID askID, const int soundID, bool selection, bool cancel) override; //Show a dialog, player must take decision. If selection then he has to choose between one of given components, if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called with number of selected component (1 - n) or 0 for cancel (if allowed) and askID.
 	void showGarrisonDialog(const CArmedInstance * up, const CGHeroInstance * down, bool removableUnits, QueryID queryID) override; //all stacks operations between these objects become allowed, interface has to call onEnd when done
-	void showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
+	void showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
 	void showMapObjectSelectDialog(QueryID askID, const Component & icon, const MetaString & title, const MetaString & description, const std::vector<ObjectInstanceID> & objects) override;
 	void saveGame(BinarySerializer & h, const int version) override; //saving
 	void loadGame(BinaryDeserializer & h, const int version) override; //loading

client/CPlayerInterface.cpp

@@ -131,7 +131,8 @@ struct HeroObjectRetriever
 };
 
 CPlayerInterface::CPlayerInterface(PlayerColor Player):
-	localState(std::make_unique<PlayerLocalState>(*this))
+	localState(std::make_unique<PlayerLocalState>(*this)),
+	movementController(std::make_unique<HeroMovementController>())
 {
 	logGlobal->trace("\tHuman player interface for player %s being constructed", Player.toString());
 	GH.defActionsDef = 0;
@@ -935,6 +936,9 @@ void CPlayerInterface::showInfoDialog(EInfoWindowMode type, const std::string &t
 		waitWhileDialog(); //Fix for mantis #98
 		adventureInt->showInfoBoxMessage(components, text, timer);
 
+		// abort movement, if any. Strictly speaking unnecessary, but prevents some edge cases, like movement sound on visiting Magi Hut with "show messages in status window" on
+		movementController->movementAbortRequested();
+
 		if (makingTurn && GH.windows().count() > 0 && LOCPLINT == this)
 			CCS->soundh->playSound(static_cast<soundBase::soundID>(soundID));
 		return;
@@ -980,7 +984,7 @@ void CPlayerInterface::showInfoDialog(const std::string &text, const std::vector
 	{
 		CCS->soundh->playSound(static_cast<soundBase::soundID>(soundID));
 		showingDialog->set(true);
-		movementController->movementStopRequested(); // interrupt movement to show dialog
+		movementController->movementAbortRequested(); // interrupt movement to show dialog
 		GH.windows().pushWindow(temp);
 	}
 	else
@@ -1003,7 +1007,7 @@ void CPlayerInterface::showYesNoDialog(const std::string &text, CFunctionList<vo
 {
 	boost::unique_lock<boost::recursive_mutex> un(*pim);
 
-	movementController->movementStopRequested();
+	movementController->movementAbortRequested();
 	LOCPLINT->showingDialog->setn(true);
 	CInfoWindow::showYesNoDialog(text, components, onYes, onNo, playerID);
 }
@@ -1013,7 +1017,7 @@ void CPlayerInterface::showBlockingDialog( const std::string &text, const std::v
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	waitWhileDialog();
 
-	movementController->movementStopRequested();
+	movementController->movementAbortRequested();
 	CCS->soundh->playSound(static_cast<soundBase::soundID>(soundID));
 
 	if (!selection && cancel) //simple yes/no dialog
@@ -1046,10 +1050,10 @@ void CPlayerInterface::showBlockingDialog( const std::string &text, const std::v
 	}
 }
 
-void CPlayerInterface::showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
+void CPlayerInterface::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
-	movementController->showTeleportDialog(channel, exits, impassable, askID);
+	movementController->showTeleportDialog(hero, channel, exits, impassable, askID);
 }
 
 void CPlayerInterface::showMapObjectSelectDialog(QueryID askID, const Component & icon, const MetaString & title, const MetaString & description, const std::vector<ObjectInstanceID> & objects)
@@ -1163,7 +1167,7 @@ void CPlayerInterface::moveHero( const CGHeroInstance *h, const CGPath& path )
 	assert(LOCPLINT->makingTurn);
 	assert(h);
 	assert(!showingDialog->get());
-	assert(!dialogs.empty());
+	assert(dialogs.empty());
 
 	LOG_TRACE(logGlobal);
 	if (!LOCPLINT->makingTurn)
@@ -1230,16 +1234,10 @@ void CPlayerInterface::showArtifactAssemblyDialog(const Artifact * artifact, con
 
 void CPlayerInterface::requestRealized( PackageApplied *pa )
 {
-	EVENT_HANDLER_CALLED_BY_CLIENT;
-
 	if(pa->packType == typeList.getTypeID<MoveHero>())
 		movementController->onMoveHeroApplied();
-
-	if (pa->packType == typeList.getTypeID<QueryReply>())
-		movementController->onQueryReplyApplied();
 }
 
-
 void CPlayerInterface::showHeroExchange(ObjectInstanceID hero1, ObjectInstanceID hero2)
 {
 	heroExchangeStarted(hero1, hero2, QueryID(-1));

client/CPlayerInterface.h

@@ -120,7 +120,7 @@ protected: // Call-ins from server, should not be called directly, but only via
 	void showInfoDialog(EInfoWindowMode type, const std::string & text, const std::vector<Component> & components, int soundID) override;
 	void showRecruitmentDialog(const CGDwelling *dwelling, const CArmedInstance *dst, int level) override;
 	void showBlockingDialog(const std::string &text, const std::vector<Component> &components, QueryID askID, const int soundID, bool selection, bool cancel) override; //Show a dialog, player must take decision. If selection then he has to choose between one of given components, if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called with number of selected component (1 - n) or 0 for cancel (if allowed) and askID.
-	void showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
+	void showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) override;
 	void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, QueryID queryID) override;
 	void showMapObjectSelectDialog(QueryID askID, const Component & icon, const MetaString & title, const MetaString & description, const std::vector<ObjectInstanceID> & objects) override;
 	void showMarketWindow(const IMarket *market, const CGHeroInstance *visitor) override;

client/HeroMovementController.cpp

@@ -30,45 +30,31 @@
 #include "../lib/NetPacks.h"
 #include "../lib/CondSh.h"
 
-// Code flow of movement process:
+std::optional<int3> HeroMovementController::getLastTile(const CGHeroInstance * hero) const
-// - Player requests movement (e.g. press "move" button)
-// -> calls doMoveHero (GUI thread)
-// -> -> sends LOCPLINT->cb->moveHero
-// - Server sends reply
-// -> calls heroMoved (NETWORK thread)
-// -> -> animation starts, sound starts
-// -> -> waits for animation to finish
-// -> -> if player have requested stop and path is not finished, calls doMoveHero again
-//
-// - Players requests movement stop (e.g. mouse click during movement)
-// -> calls movementStopRequested
-// -> -> sets flag that movement should be aborted
-
-HeroMovementController::HeroMovementController()
 {
-	destinationTeleport = ObjectInstanceID();
+	if (!LOCPLINT->localState->hasPath(hero))
-	destinationTeleportPos = int3(-1);
+		return std::nullopt;
-	duringMovement = false;
+
+	return LOCPLINT->localState->getPath(hero).endPos();
 }
 
-void HeroMovementController::setMovementStatus(bool value)
+std::optional<int3> HeroMovementController::getNextTile(const CGHeroInstance * hero) const
 {
-	duringMovement = value;
+	if (!LOCPLINT->localState->hasPath(hero))
-	if (value)
+		return std::nullopt;
-		CCS->curh->hide();
+
-	else
+	return LOCPLINT->localState->getPath(hero).nextNode().coord;
-		CCS->curh->show();
 }
 
 bool HeroMovementController::isHeroMovingThroughGarrison(const CGHeroInstance * hero, const CArmedInstance * garrison) const
 {
 	assert(LOCPLINT->localState->hasPath(hero));
-	assert(movementState == EMoveState::DURING_MOVE);
+	assert(duringMovement);
 
 	if (!LOCPLINT->localState->hasPath(hero))
 		return false;
 
-	if (garrison->visitableAt(*LOCPLINT->localState->getLastTile(hero)))
+	if (garrison->visitableAt(*getLastTile(hero)))
 		return false; // hero want to enter garrison, not pass through it
 
 	return true;
@@ -79,55 +65,50 @@ bool HeroMovementController::isHeroMoving() const
 	return duringMovement;
 }
 
-void HeroMovementController::onMoveHeroApplied()
-{
-	assert(movementState == EMoveState::DURING_MOVE);
-
-	if(movementState == EMoveState::DURING_MOVE)
-	{
-		assert(destinationTeleport == ObjectInstanceID::NONE);
-		movementState = EMoveState::CONTINUE_MOVE;
-	}
-}
-
-void HeroMovementController::onQueryReplyApplied()
-{
-	assert(movementState == EMoveState::DURING_MOVE);
-
-	if(movementState == EMoveState::DURING_MOVE)
-	{
-		// After teleportation via CGTeleport object is finished
-		assert(destinationTeleport != ObjectInstanceID::NONE);
-		destinationTeleport = ObjectInstanceID();
-		destinationTeleportPos = int3(-1);
-		movementState = EMoveState::CONTINUE_MOVE;
-	}
-}
-
 void HeroMovementController::onPlayerTurnStarted()
 {
-	assert(movementState == EMoveState::STOP_MOVE);
+	assert(duringMovement == false);
-	movementState = EMoveState::STOP_MOVE;
+	assert(stoppingMovement == false);
+	duringMovement = false;
 }
 
 void HeroMovementController::onBattleStarted()
 {
-	assert(movementState == EMoveState::STOP_MOVE);
 	// when battle starts, game will send battleStart pack *before* movement confirmation
 	// and since network thread wait for battle intro to play, movement confirmation will only happen after intro
 	// leading to several bugs, such as blocked input during intro
-	movementState = EMoveState::STOP_MOVE;
+	// FIXME: should be on hero visit
+	assert(duringMovement == false);
+	assert(stoppingMovement == false);
+	duringMovement = false;
 }
 
-void HeroMovementController::showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
+void HeroMovementController::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID)
 {
-	assert(destinationTeleport != ObjectInstanceID::NONE);
+	if (!LOCPLINT->localState->hasPath(hero))
+	{
+		assert(exits.size() == 1); // select random? Let server select?
+		LOCPLINT->cb->selectionMade(0, askID);
+		return;
+	}
 
-	auto neededExit = std::make_pair(destinationTeleport, destinationTeleportPos);
+	const auto & heroPath = LOCPLINT->localState->getPath(hero);
-	assert(vstd::contains(exits, neededExit));
+	const auto & nextNode = heroPath.nextNode();
 
-	int choosenExit = vstd::find_pos(exits, neededExit);
+	for (size_t i = 0; i < exits.size(); ++i)
-	LOCPLINT->cb->selectionMade(choosenExit, askID);
+	{
+		const auto * teleporter = LOCPLINT->cb->getObj(exits[i].first);
+
+		if (teleporter && teleporter->visitableAt(nextNode.coord))
+		{
+			LOCPLINT->cb->selectionMade(i, askID);
+			return;
+		}
+	}
+
+	assert(0); // exit not found? How? select random? Let server select?
+	LOCPLINT->cb->selectionMade(0, askID);
+	return;
 }
 
 void HeroMovementController::updatePath(const CGHeroInstance * hero, const TryMoveHero & details)
@@ -135,13 +116,15 @@ void HeroMovementController::updatePath(const CGHeroInstance * hero, const TryMo
 	if (hero->tempOwner != LOCPLINT->playerID)
 		return;
 
+	if (!LOCPLINT->localState->hasPath(hero))
+		return; // may happen when hero teleports
+
 	assert(LOCPLINT->makingTurn);
-	assert(LOCPLINT->localState->hasPath(hero));
+	assert(getNextTile(hero).has_value());
-	assert(LOCPLINT->localState->getNextTile(hero).has_value());
 
-	bool directlyAttackingCreature = details.attackedFrom.has_value() && LOCPLINT->localState->getLastTile(hero) == details.attackedFrom;
+	bool directlyAttackingCreature = details.attackedFrom.has_value() && getLastTile(hero) == details.attackedFrom;
 
-	auto desiredTarget = LOCPLINT->localState->getNextTile(hero);
+	auto desiredTarget = getNextTile(hero);
 	auto actualTarget = hero->convertToVisitablePos(details.end);
 
 	//don't erase path when revisiting with spacebar
@@ -168,6 +151,8 @@ void HeroMovementController::updatePath(const CGHeroInstance * hero, const TryMo
 
 void HeroMovementController::heroMoved(const CGHeroInstance * hero, const TryMoveHero & details)
 {
+	//assert(duringMovement == true); // May be false if hero teleported
+
 	if (details.result == TryMoveHero::EMBARK || details.result == TryMoveHero::DISEMBARK)
 	{
 		if(hero->getRemovalSound() && hero->tempOwner == LOCPLINT->playerID)
@@ -185,8 +170,8 @@ void HeroMovementController::heroMoved(const CGHeroInstance * hero, const TryMov
 
 	if(details.stopMovement()) //hero failed to move
 	{
-		movementState = EMoveState::STOP_MOVE;
+		if (duringMovement)
-		adventureInt->onHeroChanged(hero);
+			endHeroMove(hero);
 		return;
 	}
 
@@ -195,13 +180,6 @@ void HeroMovementController::heroMoved(const CGHeroInstance * hero, const TryMov
 	//move finished
 	adventureInt->onHeroChanged(hero);
 
-	//check if user cancelled movement
-	if (GH.input().ignoreEventsUntilInput())
-		movementState = EMoveState::STOP_MOVE;
-
-	if (movementState == EMoveState::WAITING_MOVE)
-		movementState = EMoveState::DURING_MOVE;
-
 //	// Hero attacked creature directly, set direction to face it.
 //	if (directlyAttackingCreature)
 //	{
@@ -218,12 +196,51 @@ void HeroMovementController::heroMoved(const CGHeroInstance * hero, const TryMov
 //	}
 }
 
-void HeroMovementController::movementStopRequested()
+void HeroMovementController::onMoveHeroApplied()
 {
-	assert(movementState == EMoveState::DURING_MOVE);
+	auto const * hero = LOCPLINT->localState->getCurrentHero();
 
-	if (movementState == EMoveState::DURING_MOVE)//if we are in the middle of hero movement
+	assert(hero);
-		movementState = EMoveState::STOP_MOVE;
+
+	//check if user cancelled movement
+	if (GH.input().ignoreEventsUntilInput())
+		stoppingMovement = true;
+
+	if (duringMovement)
+	{
+		bool canMove = LOCPLINT->localState->hasPath(hero) && LOCPLINT->localState->getPath(hero).nextNode().turns == 0;
+		bool wantStop = stoppingMovement;
+		bool canStop = !canMove || canHeroStopAtNode(LOCPLINT->localState->getPath(hero).currNode());
+
+		if (!canMove)
+		{
+			endHeroMove(hero);
+		}
+		else if ( wantStop && canStop )
+		{
+			endHeroMove(hero);
+		}
+		else
+		{
+			moveHeroOnce(hero, LOCPLINT->localState->getPath(hero));
+		}
+	}
+}
+
+void HeroMovementController::movementAbortRequested()
+{
+	if(duringMovement)
+		endHeroMove(LOCPLINT->localState->getCurrentHero());
+}
+
+void HeroMovementController::endHeroMove(const CGHeroInstance * hero)
+{
+	assert(duringMovement == true);
+	duringMovement = false;
+	stoppingMovement = false;
+	stopMovementSound();
+	adventureInt->onHeroChanged(hero);
+	CCS->curh->show();
 }
 
 AudioPath HeroMovementController::getMovementSoundFor(const CGHeroInstance * hero, int3 posPrev, int3 posNext, EPathNodeAction moveType)
@@ -241,8 +258,8 @@ AudioPath HeroMovementController::getMovementSoundFor(const CGHeroInstance * her
 	if (hero->hasBonusOfType(BonusType::FLYING_MOVEMENT))
 		return AudioPath::builtin("HORSE10.wav");
 
-	auto prevTile = LOCPLINT->cb->getTile(hero->convertToVisitablePos(posPrev));
+	auto prevTile = LOCPLINT->cb->getTile(posPrev);
-	auto nextTile = LOCPLINT->cb->getTile(hero->convertToVisitablePos(posNext));
+	auto nextTile = LOCPLINT->cb->getTile(posNext);
 
 	auto prevRoad = prevTile->roadType;
 	auto nextRoad = nextTile->roadType;
@@ -254,155 +271,93 @@ AudioPath HeroMovementController::getMovementSoundFor(const CGHeroInstance * her
 		return nextTile->terType->horseSoundPenalty;
 };
 
-void HeroMovementController::updateMovementSound(const CGHeroInstance * hero, int3 posPrev, int3 posNext, EPathNodeAction moveType)
+void HeroMovementController::updateMovementSound(const CGHeroInstance * h, int3 posPrev, int3 nextCoord, EPathNodeAction moveType)
 {
-
 	// Start a new sound for the hero movement or let the existing one carry on.
-	AudioPath newSoundName = getMovementSoundFor(h, prevCoord, nextCoord, path.nodes[i-1].action);
+	AudioPath newSoundName = getMovementSoundFor(h, posPrev, nextCoord, moveType);
 
-	if(newSoundName != soundName)
+	if(newSoundName != currentMovementSoundName)
 	{
-		soundName = newSoundName;
+		currentMovementSoundName = newSoundName;
 
-		CCS->soundh->stopSound(soundChannel);
+		if (currentMovementSoundChannel != -1)
-		if (!soundName.empty())
+			CCS->soundh->stopSound(currentMovementSoundChannel);
-			soundChannel = CCS->soundh->playSound(soundName, -1);
+
+		if (!currentMovementSoundName.empty())
+			currentMovementSoundChannel = CCS->soundh->playSound(currentMovementSoundName, -1, true);
 		else
-			soundChannel = -1;
+			currentMovementSoundChannel = -1;
 	}
-
-
-	int soundChannel = -1;
-	AudioPath soundName;
 }
 
 void HeroMovementController::stopMovementSound()
 {
-	CCS->soundh->stopSound(soundChannel);
+	CCS->soundh->stopSound(currentMovementSoundChannel);
+	currentMovementSoundChannel = -1;
+	currentMovementSoundName = AudioPath();
+}
+
+bool HeroMovementController::canHeroStopAtNode(const CGPathNode & node) const
+{
+	if (node.layer != EPathfindingLayer::LAND && node.layer != EPathfindingLayer::SAIL)
+		return false;
+
+	if (node.accessible != EPathAccessibility::ACCESSIBLE)
+		return false;
+
+	return true;
 }
 
 void HeroMovementController::movementStartRequested(const CGHeroInstance * h, const CGPath & path)
 {
-	setMovementStatus(true);
+	assert(duringMovement == false);
+	duringMovement = true;
 
-	auto getObj = [&](int3 coord, bool ignoreHero)
+	CCS->curh->show();
-	{
+	moveHeroOnce(h, path);
-		return LOCPLINT->cb->getTile(h->convertToVisitablePos(coord))->topVisitableObj(ignoreHero);
+}
-	};
+
-
+void HeroMovementController::moveHeroOnce(const CGHeroInstance * h, const CGPath & path)
-	auto isTeleportAction = [&](EPathNodeAction action) -> bool
+{
-	{
+	assert(duringMovement == true);
-		if (action != EPathNodeAction::TELEPORT_NORMAL &&
+
-			action != EPathNodeAction::TELEPORT_BLOCKING_VISIT &&
+	const auto & currNode = path.currNode();
-			action != EPathNodeAction::TELEPORT_BATTLE)
+	const auto & nextNode = path.nextNode();
-		{
+
-			return false;
+	assert(nextNode.turns == 0);
-		}
+	assert(currNode.coord == h->visitablePos());
-
+
-		return true;
+	int3 nextCoord = h->convertFromVisitablePos(nextNode.coord);
-	};
+
-
+	if (nextNode.isTeleportAction())
-	auto getDestTeleportObj = [&](const CGObjectInstance * currentObject, const CGObjectInstance * nextObjectTop, const CGObjectInstance * nextObject) -> const CGObjectInstance *
+	{
-	{
+		stopMovementSound();
-		if (CGTeleport::isConnected(currentObject, nextObjectTop))
+		LOCPLINT->cb->moveHero(h, nextCoord, false);
-			return nextObjectTop;
+		return;
-
+	}
-		if (nextObjectTop && nextObjectTop->ID == Obj::HERO && CGTeleport::isConnected(currentObject, nextObject))
+	else
-		{
+	{
-			return nextObject;
+		updateMovementSound(h, currNode.coord, nextNode.coord, nextNode.action);
-		}
+
-
+		assert(h->pos.z == nextNode.coord.z); // Z should change only if it's movement via teleporter and in this case this code shouldn't be executed at all
-		return nullptr;
+
-	};
+		logGlobal->trace("Requesting hero movement to %s", nextNode.coord.toString());
-
+
-	auto doMovement = [&](int3 dst, bool transit)
+		bool useTransit = nextNode.layer == EPathfindingLayer::AIR;
-	{
+		LOCPLINT->cb->moveHero(h, nextCoord, useTransit);
-		movementState = EMoveState::WAITING_MOVE;
+		return;
-		LOCPLINT->cb->moveHero(h, dst, transit);
+	}
-		while(movementState != EMoveState::STOP_MOVE && movementState != EMoveState::CONTINUE_MOVE)
+
-		{
+//	bool guarded = LOCPLINT->cb->isInTheMap(LOCPLINT->cb->getGuardingCreaturePosition(nextCoord - int3(1, 0, 0)));
-			assert(0);
+//	if ((!useTransit && guarded) || LOCPLINT->showingDialog->get() == true) // Abort movement if a guard was fought or there is a dialog to display (Mantis #1136)
-			boost::this_thread::sleep_for(boost::chrono::milliseconds(1));
+//		break;
-		}
+
-	};
+//	stopMovementSound();
-
+//
-	auto canStop = [&](const CGPathNode * node) -> bool
+//	//Update cursor so icon can change if needed when it reappears; doesn;'t apply if a dialog box pops up at the end of the movement
-	{
+//	if (!LOCPLINT->showingDialog->get())
-		if (node->layer != EPathfindingLayer::LAND && node->layer != EPathfindingLayer::SAIL)
+//		GH.fakeMouseMove();
-			return false;
+//
-
+//	CGI->mh->waitForOngoingAnimations();
-		if (node->accessible != EPathAccessibility::ACCESSIBLE)
+//	setMovementStatus(false);
-			return false;
-
-		return true;
-	};
-
-	int i = 1;
-	movementState = EMoveState::CONTINUE_MOVE;
-
-	for (i=(int)path.nodes.size()-1; i>0 && (movementState == EMoveState::CONTINUE_MOVE || !canStop(&path.nodes[i])); i--)
-	{
-		int3 prevCoord = h->convertFromVisitablePos(path.nodes[i].coord);
-		int3 nextCoord = h->convertFromVisitablePos(path.nodes[i-1].coord);
-
-		auto prevObject = getObj(prevCoord, prevCoord == h->pos);
-		auto nextObjectTop = getObj(nextCoord, false);
-		auto nextObject = getObj(nextCoord, true);
-		auto destTeleportObj = getDestTeleportObj(prevObject, nextObjectTop, nextObject);
-		if (isTeleportAction(path.nodes[i-1].action) && destTeleportObj != nullptr)
-		{
-			stopMovementSound();
-			destinationTeleport = destTeleportObj->id;
-			destinationTeleportPos = nextCoord;
-			doMovement(h->pos, false);
-			if (path.nodes[i-1].action == EPathNodeAction::TELEPORT_BLOCKING_VISIT
-				|| path.nodes[i-1].action == EPathNodeAction::TELEPORT_BATTLE)
-			{
-				destinationTeleport = ObjectInstanceID();
-				destinationTeleportPos = int3(-1);
-			}
-
-			if(i != path.nodes.size() - 1)
-				updateMovementSound(h, prevCoord, nextCoord, path.nodes[i-1].action);
-
-			continue;
-		}
-
-		if (path.nodes[i-1].turns)
-		{ //stop sending move requests if the next node can't be reached at the current turn (hero exhausted his move points)
-			movementState = EMoveState::STOP_MOVE;
-			break;
-		}
-
-		updateMovementSound(h, prevCoord, nextCoord, path.nodes[i-1].action);
-
-		assert(h->pos.z == nextCoord.z); // Z should change only if it's movement via teleporter and in this case this code shouldn't be executed at all
-		int3 endpos(nextCoord.x, nextCoord.y, h->pos.z);
-		logGlobal->trace("Requesting hero movement to %s", endpos.toString());
-
-		bool useTransit = false;
-		if ((i-2 >= 0) // Check there is node after next one; otherwise transit is pointless
-			&& (CGTeleport::isConnected(nextObjectTop, getObj(h->convertFromVisitablePos(path.nodes[i-2].coord), false))
-				|| CGTeleport::isTeleport(nextObjectTop)))
-		{ // Hero should be able to go through object if it's allow transit
-			useTransit = true;
-		}
-		else if (path.nodes[i-1].layer == EPathfindingLayer::AIR)
-			useTransit = true;
-
-		doMovement(endpos, useTransit);
-
-		logGlobal->trace("Resuming %s", __FUNCTION__);
-		bool guarded = LOCPLINT->cb->isInTheMap(LOCPLINT->cb->getGuardingCreaturePosition(endpos - int3(1, 0, 0)));
-		if ((!useTransit && guarded) || LOCPLINT->showingDialog->get() == true) // Abort movement if a guard was fought or there is a dialog to display (Mantis #1136)
-			break;
-	}
-	stopMovementSound();
-
-	//Update cursor so icon can change if needed when it reappears; doesn;'t apply if a dialog box pops up at the end of the movement
-	if (!LOCPLINT->showingDialog->get())
-		GH.fakeMouseMove();
-
-	CGI->mh->waitForOngoingAnimations();
-	setMovementStatus(false);
 }

client/HeroMovementController.h

@@ -18,6 +18,7 @@ using TTeleportExitsList = std::vector<std::pair<ObjectInstanceID, int3>>;
 
 class CGHeroInstance;
 class CArmedInstance;
+struct CGPathNode;
 
 struct CGPath;
 struct TryMoveHero;
@@ -26,45 +27,45 @@ VCMI_LIB_NAMESPACE_END
 
 class HeroMovementController
 {
-	ObjectInstanceID destinationTeleport; //contain -1 or object id if teleportation
+	/// there is an ongoing movement loop, in one or another stage
-	int3 destinationTeleportPos;
+	bool duringMovement = false;
+	/// movement was requested to be terminated, e.g. by player or due to inability to move
+	bool stoppingMovement = false;
 
-	bool duringMovement;
+	AudioPath currentMovementSoundName;
+	int currentMovementSoundChannel = -1;
 
+	/// return final node in a path, if exists
+	std::optional<int3> getLastTile(const CGHeroInstance * h) const;
+	/// return first path in a path, if exists
+	std::optional<int3> getNextTile(const CGHeroInstance * h) const;
 
-	enum class EMoveState
+	bool canHeroStopAtNode(const CGPathNode & node) const;
-	{
-		STOP_MOVE,
-		WAITING_MOVE,
-		CONTINUE_MOVE,
-		DURING_MOVE
-	};
-
-	EMoveState movementState;
-
-	void setMovementStatus(bool value);
 
 	void updatePath(const CGHeroInstance * hero, const TryMoveHero & details);
 
+	/// Moves hero 1 tile / path node
+	void moveHeroOnce(const CGHeroInstance * h, const CGPath & path);
+
+	void endHeroMove(const CGHeroInstance * h);
+
 	AudioPath getMovementSoundFor(const CGHeroInstance * hero, int3 posPrev, int3 posNext, EPathNodeAction moveType);
 	void updateMovementSound(const CGHeroInstance * hero, int3 posPrev, int3 posNext, EPathNodeAction moveType);
 	void stopMovementSound();
-public:
-	HeroMovementController();
 
+public:
 	// const queries
 	bool isHeroMovingThroughGarrison(const CGHeroInstance * hero, const CArmedInstance * garrison) const;
 	bool isHeroMoving() const;
 
 	// netpack handlers
 	void onMoveHeroApplied();
-	void onQueryReplyApplied();
 	void onPlayerTurnStarted();
 	void onBattleStarted();
-	void showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID);
+	void showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID);
 	void heroMoved(const CGHeroInstance * hero, const TryMoveHero & details);
 
 	// UI handlers
 	void movementStartRequested(const CGHeroInstance * h, const CGPath & path);
-	void movementStopRequested();
+	void movementAbortRequested();
 };

client/NetPacksClient.cpp

@@ -703,7 +703,8 @@ void ApplyClientNetPackVisitor::visitExchangeDialog(ExchangeDialog & pack)
 
 void ApplyClientNetPackVisitor::visitTeleportDialog(TeleportDialog & pack)
 {
-	callOnlyThatInterface(cl, pack.player, &CGameInterface::showTeleportDialog, pack.channel, pack.exits, pack.impassable, pack.queryID);
+	const CGHeroInstance *h = cl.getHero(pack.hero);
+	callOnlyThatInterface(cl, h->getOwner(), &CGameInterface::showTeleportDialog, h, pack.channel, pack.exits, pack.impassable, pack.queryID);
 }
 
 void ApplyClientNetPackVisitor::visitMapObjectSelectDialog(MapObjectSelectDialog & pack)

client/PlayerLocalState.h

@@ -76,11 +76,6 @@ public:
 	void setPath(const CGHeroInstance * h, const CGPath & path);
 	bool setPath(const CGHeroInstance * h, const int3 & destination);
 
-	/// return final node in a path, if exists
-	std::optional<int3> getLastTile(const CGHeroInstance * h) const;
-	/// return first path in a path, if exists
-	std::optional<int3> getNextTile(const CGHeroInstance * h) const;
-
 	const CGPath & getPath(const CGHeroInstance * h) const;
 	bool hasPath(const CGHeroInstance * h) const;
 

client/adventureMap/AdventureMapInterface.cpp

@@ -531,7 +531,8 @@ void AdventureMapInterface::onTileLeftClicked(const int3 &mapPos)
 			if(LOCPLINT->localState->hasPath(currentHero) &&
 			   LOCPLINT->localState->getPath(currentHero).endPos() == mapPos)//we'll be moving
 			{
-				if(!CGI->mh->hasOngoingAnimations())
+				assert(!CGI->mh->hasOngoingAnimations());
+				if(!CGI->mh->hasOngoingAnimations() && LOCPLINT->localState->getPath(currentHero).nextNode().turns == 0)
 					LOCPLINT->moveHero(currentHero, LOCPLINT->localState->getPath(currentHero));
 				return;
 			}

lib/CGameInterface.h

@@ -103,7 +103,7 @@ public:
 
 	// all stacks operations between these objects become allowed, interface has to call onEnd when done
 	virtual void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, QueryID queryID) = 0;
-	virtual void showTeleportDialog(TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) = 0;
+	virtual void showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID channel, TTeleportExitsList exits, bool impassable, QueryID askID) = 0;
 	virtual void showMapObjectSelectDialog(QueryID askID, const Component & icon, const MetaString & title, const MetaString & description, const std::vector<ObjectInstanceID> & objects) = 0;
 	virtual void finish(){}; //if for some reason we want to end
 

lib/NetPacks.h

@@ -1436,12 +1436,12 @@ struct DLL_LINKAGE TeleportDialog : public Query
 {
 	TeleportDialog() = default;
 
-	TeleportDialog(const PlayerColor & Player, const TeleportChannelID & Channel)
+	TeleportDialog(const ObjectInstanceID & hero, const TeleportChannelID & Channel)
-		: player(Player)
+		: hero(hero)
 		, channel(Channel)
 	{
 	}
-	PlayerColor player;
+	ObjectInstanceID hero;
 	TeleportChannelID channel;
 	TTeleportExitsList exits;
 	bool impassable = false;
@@ -1451,7 +1451,7 @@ struct DLL_LINKAGE TeleportDialog : public Query
 	template <typename Handler> void serialize(Handler & h, const int version)
 	{
 		h & queryID;
-		h & player;
+		h & hero;
 		h & channel;
 		h & exits;
 		h & impassable;

lib/mapObjects/MiscObjects.cpp

@@ -453,7 +453,7 @@ TeleportChannelID CGMonolith::findMeChannel(const std::vector<Obj> & IDs, int Su
 
 void CGMonolith::onHeroVisit( const CGHeroInstance * h ) const
 {
-	TeleportDialog td(h->tempOwner, channel);
+	TeleportDialog td(h->id, channel);
 	if(isEntrance())
 	{
 		if(cb->isTeleportChannelBidirectional(channel) && 1 < cb->getTeleportChannelExits(channel).size())
@@ -527,7 +527,7 @@ void CGMonolith::initObj(CRandomGenerator & rand)
 
 void CGSubterraneanGate::onHeroVisit( const CGHeroInstance * h ) const
 {
-	TeleportDialog td(h->tempOwner, channel);
+	TeleportDialog td(h->id, channel);
 	if(cb->isTeleportChannelImpassable(channel))
 	{
 		h->showInfoDialog(153);//Just inside the entrance you find a large pile of rubble blocking the tunnel. You leave discouraged.
@@ -611,7 +611,7 @@ void CGSubterraneanGate::postInit() //matches subterranean gates into pairs
 
 void CGWhirlpool::onHeroVisit( const CGHeroInstance * h ) const
 {
-	TeleportDialog td(h->tempOwner, channel);
+	TeleportDialog td(h->id, channel);
 	if(cb->isTeleportChannelImpassable(channel))
 	{
 		logGlobal->debug("Cannot find exit whirlpool for %d at %s", id.getNum(), pos.toString());

lib/pathfinder/CGPathNode.cpp

@@ -24,6 +24,24 @@ static bool canSeeObj(const CGObjectInstance * obj)
 	return obj != nullptr && obj->ID != Obj::EVENT;
 }
 
+const CGPathNode & CGPath::currNode() const
+{
+	assert(nodes.size() > 1);
+	return nodes[nodes.size()-1];
+}
+
+const CGPathNode & CGPath::nextNode() const
+{
+	assert(nodes.size() > 1);
+	return nodes[nodes.size()-2];
+}
+
+const CGPathNode & CGPath::lastNode() const
+{
+	assert(nodes.size() > 1);
+	return nodes[0];
+}
+
 int3 CGPath::startPos() const
 {
 	return nodes[nodes.size()-1].coord;

lib/pathfinder/CGPathNode.h

@@ -143,6 +143,18 @@ struct DLL_LINKAGE CGPathNode
 		return turns < 255;
 	}
 
+	bool isTeleportAction() const
+	{
+		if (action != EPathNodeAction::TELEPORT_NORMAL &&
+			action != EPathNodeAction::TELEPORT_BLOCKING_VISIT &&
+			action != EPathNodeAction::TELEPORT_BATTLE)
+		{
+			return false;
+		}
+
+		return true;
+	}
+
 	using TFibHeap = boost::heap::fibonacci_heap<CGPathNode *, boost::heap::compare<NodeComparer<CGPathNode>>>;
 
 	TFibHeap::handle_type pqHandle;
@@ -156,6 +168,13 @@ struct DLL_LINKAGE CGPath
 {
 	std::vector<CGPathNode> nodes; //just get node by node
 
+	/// Starting position of path, matches location of hero
+	const CGPathNode & currNode() const;
+	/// First node in path, this is where hero will move next
+	const CGPathNode & nextNode() const;
+	/// Last node in path, this is what hero wants to reach in the end
+	const CGPathNode & lastNode() const;
+
 	int3 startPos() const; // start point
 	int3 endPos() const; //destination point
 };

server/queries/MapQueries.cpp

@@ -171,7 +171,7 @@ CTeleportDialogQuery::CTeleportDialogQuery(CGameHandler * owner, const TeleportD
 	CDialogQuery(owner)
 {
 	this->td = td;
-	addPlayer(td.player);
+	addPlayer(gh->getHero(td.hero)->getOwner());
 }
 
 CHeroLevelUpDialogQuery::CHeroLevelUpDialogQuery(CGameHandler * owner, const HeroLevelUp & Hlu, const CGHeroInstance * Hero):