Moved selection logic to player state

client/CPlayerInterface.cpp

@@ -405,6 +405,7 @@ void CPlayerInterface::heroKilled(const CGHeroInstance* hero)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	LOG_TRACE_PARAMS(logGlobal, "Hero %s killed handler for player %s", hero->getNameTranslated() % playerID);
+
 	localState->removeWanderingHero(hero);
 	adventureInt->onHeroChanged(hero);
 	localState->erasePath(hero);
@@ -2084,12 +2085,3 @@ void CPlayerInterface::showWorldViewEx(const std::vector<ObjectPosInfo>& objectP
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	adventureInt->openWorldView(objectPositions, showTerrain );
 }
-
-void CPlayerInterface::setSelection(const CArmedInstance *sel, bool centerView)
-{
-	if (sel == localState->getCurrentArmy())
-		return;
-
-	localState->setSelection(sel);
-	adventureInt->onSelectionChanged(sel, centerView);
-}

client/CPlayerInterface.h

@@ -207,9 +207,6 @@ public: // public interface for use by client via LOCPLINT access
 	void showShipyardDialogOrProblemPopup(const IShipyard *obj); //obj may be town or shipyard;
 	void proposeLoadingGame();
 
-	/// Changes currently selected object
-	void setSelection(const CArmedInstance *sel, bool centerView = true);
-
 	///returns true if all events are processed internally
 	bool capturedAllEvents();
 

client/PlayerLocalState.cpp

@@ -107,6 +107,41 @@ const CGHeroInstance * PlayerLocalState::getCurrentHero() const
 		return nullptr;
 }
 
+const CGHeroInstance * PlayerLocalState::getNextWanderingHero(const CGHeroInstance * currentHero)
+{
+	bool currentHeroFound = false;
+	const CGHeroInstance * firstSuitable = nullptr;
+	const CGHeroInstance * nextSuitable = nullptr;
+
+	for(const auto * hero : getWanderingHeroes())
+	{
+		if (hero == currentHero)
+		{
+			currentHeroFound = true;
+			continue;
+		}
+
+		if (isHeroSleeping(hero))
+			continue;
+
+		if (hero->movement == 0)
+			continue;
+
+		if (!firstSuitable)
+			firstSuitable = hero;
+
+		if (!nextSuitable && currentHeroFound)
+			nextSuitable = hero;
+	}
+
+	// if we found suitable hero after currently selected hero -> return this hero
+	if (nextSuitable)
+		return nextSuitable;
+
+	// othervice -> loop over and return first suitable hero in the list (or null if none)
+	return firstSuitable;
+}
+
 const CGTownInstance * PlayerLocalState::getCurrentTown() const
 {
 	if(currentSelection && currentSelection->ID == Obj::TOWN)
@@ -125,7 +160,13 @@ const CArmedInstance * PlayerLocalState::getCurrentArmy() const
 
 void PlayerLocalState::setSelection(const CArmedInstance * selection)
 {
+	if (currentSelection == selection)
+		return;
+
 	currentSelection = selection;
+
+	if (selection)
+		adventureInt->onSelectionChanged(selection);
 }
 
 bool PlayerLocalState::isHeroSleeping(const CGHeroInstance * hero) const
@@ -174,8 +215,21 @@ void PlayerLocalState::removeWanderingHero(const CGHeroInstance * hero)
 {
 	assert(hero);
 	assert(vstd::contains(wanderingHeroes, hero));
+
+	if (hero == currentSelection)
+	{
+		auto const * nextHero = getNextWanderingHero(hero);
+		setSelection(nextHero);
+	}
+
 	vstd::erase(wanderingHeroes, hero);
 	vstd::erase(sleepingHeroes, hero);
+
+	if (currentSelection == nullptr && !wanderingHeroes.empty())
+		setSelection(wanderingHeroes.front());
+
+	if (currentSelection == nullptr && !ownedTowns.empty())
+		setSelection(ownedTowns.front());
 }
 
 const std::vector<const CGTownInstance *> & PlayerLocalState::getOwnedTowns()
@@ -202,4 +256,13 @@ void PlayerLocalState::removeOwnedTown(const CGTownInstance * town)
 	assert(town);
 	assert(vstd::contains(ownedTowns, town));
 	vstd::erase(ownedTowns, town);
+
+	if (town == currentSelection)
+		setSelection(nullptr);
+
+	if (currentSelection == nullptr && !wanderingHeroes.empty())
+		setSelection(wanderingHeroes.front());
+
+	if (currentSelection == nullptr && !ownedTowns.empty())
+		setSelection(ownedTowns.front());
 }

client/PlayerLocalState.h

@@ -69,6 +69,7 @@ public:
 
 	const std::vector<const CGHeroInstance *> & getWanderingHeroes();
 	const CGHeroInstance * getWanderingHero(size_t index);
+	const CGHeroInstance * getNextWanderingHero(const CGHeroInstance * hero);
 	void addWanderingHero(const CGHeroInstance * hero);
 	void removeWanderingHero(const CGHeroInstance * hero);
 
@@ -88,7 +89,7 @@ public:
 	const CArmedInstance * getCurrentArmy() const;
 
 	/// Changes currently selected object
-	void setSelection(const CArmedInstance * selection);
+	void setSelection(const CArmedInstance *sel);
 
 	template<typename Handler>
 	void serialize(Handler & h, int version)

client/adventureMap/CAdventureMapInterface.cpp

@@ -287,9 +287,14 @@ void CAdventureMapInterface::fsleepWake()
 	if (!h)
 		return;
 	bool newSleep = !LOCPLINT->localState->isHeroSleeping(h);
-	setHeroSleeping(h, newSleep);
 
-	updateButtons();
+	if (newSleep)
+		LOCPLINT->localState->setHeroAsleep(h);
+	else
+		LOCPLINT->localState->setHeroAwaken(h);
+
+	onHeroChanged(h);
+
 	if (newSleep)
 		fnextHero();
 
@@ -328,10 +333,14 @@ void CAdventureMapInterface::fsystemOptions()
 
 void CAdventureMapInterface::fnextHero()
 {
-	const auto * nextHero = getNextHero(LOCPLINT->localState->getCurrentHero());
+	const auto * currHero = LOCPLINT->localState->getCurrentHero();
+	const auto * nextHero = LOCPLINT->localState->getNextWanderingHero(currHero);
 
 	if (nextHero)
-		LOCPLINT->setSelection(nextHero, true);
+	{
+		LOCPLINT->localState->setSelection(nextHero);
+		centerOnObject(nextHero);
+	}
 }
 
 void CAdventureMapInterface::fendTurn()
@@ -376,7 +385,7 @@ void CAdventureMapInterface::updateButtons()
 	spellbook->block(!hero);
 	moveHero->block(!hero || !LOCPLINT->localState->hasPath(hero) || hero->movement == 0);
 
-	const auto * nextSuitableHero = getNextHero(hero);
+	const auto * nextSuitableHero = LOCPLINT->localState->getNextWanderingHero(hero);
 	nextHero->block(nextSuitableHero == nullptr);
 
 	if(hero)
@@ -388,41 +397,6 @@ void CAdventureMapInterface::updateButtons()
 	}
 }
 
-const CGHeroInstance * CAdventureMapInterface::getNextHero(const CGHeroInstance * currentHero)
-{
-	bool currentHeroFound = false;
-	const CGHeroInstance * firstSuitable = nullptr;
-	const CGHeroInstance * nextSuitable = nullptr;
-
-	for(const auto * hero : LOCPLINT->localState->getWanderingHeroes())
-	{
-		if (hero == currentHero)
-		{
-			currentHeroFound = true;
-			continue;
-		}
-
-		if (LOCPLINT->localState->isHeroSleeping(hero))
-			continue;
-
-		if (hero->movement == 0)
-			continue;
-
-		if (!firstSuitable)
-			firstSuitable = hero;
-
-		if (!nextSuitable && currentHeroFound)
-			nextSuitable = hero;
-	}
-
-	// if we found suitable hero after currently selected hero -> return this hero
-	if (nextSuitable)
-		return nextSuitable;
-
-	// othervice -> loop over and return first suitable hero in the list (or null if none)
-	return firstSuitable;
-}
-
 void CAdventureMapInterface::onHeroChanged(const CGHeroInstance *h)
 {
 	heroList->update(h);
@@ -522,16 +496,6 @@ void CAdventureMapInterface::showAll(SDL_Surface * to)
 	LOCPLINT->cingconsole->show(to);
 }
 
-void CAdventureMapInterface::setHeroSleeping(const CGHeroInstance *hero, bool sleep)
-{
-	if (sleep)
-		LOCPLINT->localState->setHeroAsleep(hero);
-	else
-		LOCPLINT->localState->setHeroAwaken(hero);
-
-	onHeroChanged(hero);
-}
-
 void CAdventureMapInterface::show(SDL_Surface * to)
 {
 //	if(state != EGameState::MAKING_TURN)
@@ -818,12 +782,14 @@ std::optional<Point> CAdventureMapInterface::keyToMoveDirection(const SDL_Keycod
 	return std::nullopt;
 }
 
-void CAdventureMapInterface::onSelectionChanged(const CArmedInstance *sel, bool centerView)
+void CAdventureMapInterface::onSelectionChanged(const CArmedInstance *sel)
 {
 	assert(sel);
 
 	infoBar->popAll();
 	mapAudio->onSelectionChanged(sel);
+	bool centerView = !settings["session"]["autoSkip"].Bool();
+
 	if (centerView)
 		centerOnObject(sel);
 
@@ -944,20 +910,18 @@ void CAdventureMapInterface::onPlayerTurnStarted(PlayerColor playerID)
 		}
 	}
 
-	bool centerView = !settings["session"]["autoSkip"].Bool();
-
 	//select first hero if available.
 	if (heroToSelect != nullptr)
 	{
-		LOCPLINT->setSelection(heroToSelect, centerView);
+		LOCPLINT->localState->setSelection(heroToSelect);
 	}
 	else if (LOCPLINT->localState->getOwnedTowns().size())
 	{
-		LOCPLINT->setSelection(LOCPLINT->localState->getOwnedTown(0), centerView);
+		LOCPLINT->localState->setSelection(LOCPLINT->localState->getOwnedTown(0));
 	}
 	else
 	{
-		LOCPLINT->setSelection(LOCPLINT->localState->getWanderingHero(0), centerView);
+		LOCPLINT->localState->setSelection(LOCPLINT->localState->getWanderingHero(0));
 	}
 
 	//show new day animation and sound on infobar
@@ -1044,7 +1008,7 @@ void CAdventureMapInterface::onTileLeftClicked(const int3 &mapPos)
 		if(LOCPLINT->localState->getCurrentArmy() == topBlocking) //selected town clicked
 			LOCPLINT->openTownWindow(static_cast<const CGTownInstance*>(topBlocking));
 		else if(canSelect)
-			LOCPLINT->setSelection(static_cast<const CArmedInstance*>(topBlocking), false);
+			LOCPLINT->localState->setSelection(static_cast<const CArmedInstance*>(topBlocking));
 	}
 	else if(const CGHeroInstance * currentHero = LOCPLINT->localState->getCurrentHero()) //hero is selected
 	{
@@ -1058,7 +1022,7 @@ void CAdventureMapInterface::onTileLeftClicked(const int3 &mapPos)
 		}
 		else if(canSelect && pn->turns == 255 ) //selectable object at inaccessible tile
 		{
-			LOCPLINT->setSelection(static_cast<const CArmedInstance*>(topBlocking), false);
+			LOCPLINT->localState->setSelection(static_cast<const CArmedInstance*>(topBlocking));
 			return;
 		}
 		else //still here? we need to move hero if we clicked end of already selected path or calculate a new path otherwise

client/adventureMap/CAdventureMapInterface.h

@@ -132,8 +132,6 @@ private:
 
 	std::optional<Point> keyToMoveDirection(const SDL_Keycode & key);
 
-	void setHeroSleeping(const CGHeroInstance *hero, bool sleep);
-	const CGHeroInstance * getNextHero(const CGHeroInstance * currentHero); //for Next Hero button - cycles awake heroes with movement only
 	void endingTurn();
 
 	/// exits currently opened world view mode and returns to normal map
@@ -176,8 +174,8 @@ public:
 	/// Called by PlayerInterface when town state changed and town list must be updated
 	void onTownChanged(const CGTownInstance * town);
 
-	/// Changes currently selected object
+	/// Called when currently selected object changes
-	void onSelectionChanged(const CArmedInstance *sel, bool centerView = true);
+	void onSelectionChanged(const CArmedInstance *sel);
 
 	/// Called when map audio should be paused, e.g. on combat or town screen access
 	void onAudioPaused();

client/adventureMap/CList.cpp

@@ -204,8 +204,8 @@ std::shared_ptr<CIntObject> CHeroList::CHeroItem::genSelection()
 
 void CHeroList::CHeroItem::select(bool on)
 {
-	if(on && LOCPLINT->localState->getCurrentHero() != hero)
+	if(on)
-		LOCPLINT->setSelection(hero);
+		LOCPLINT->localState->setSelection(hero);
 }
 
 void CHeroList::CHeroItem::open()
@@ -293,8 +293,8 @@ void CTownList::CTownItem::update()
 
 void CTownList::CTownItem::select(bool on)
 {
-	if (on && LOCPLINT->localState->getCurrentTown() != town)
+	if(on)
-		LOCPLINT->setSelection(town, true);
+		LOCPLINT->localState->setSelection(town);
 }
 
 void CTownList::CTownItem::open()

client/windows/CCastleInterface.cpp

@@ -1230,9 +1230,9 @@ void CCastleInterface::close()
 	if(town->tempOwner == LOCPLINT->playerID) //we may have opened window for an allied town
 	{
 		if(town->visitingHero && town->visitingHero->tempOwner == LOCPLINT->playerID)
-			LOCPLINT->setSelection(town->visitingHero);
+			LOCPLINT->localState->setSelection(town->visitingHero);
 		else
-			LOCPLINT->setSelection(town);
+			LOCPLINT->localState->setSelection(town);
 	}
 	CWindowObject::close();
 }