Workaround for false double-clicking of elements in hero/town lists

client/adventureMap/CList.cpp

@@ -287,8 +287,28 @@ void CHeroList::updateElement(const CGHeroInstance * hero)
 
 void CHeroList::updateWidget()
 {
-	listBox->resize(LOCPLINT->localState->getWanderingHeroes().size());
+	auto & heroes = LOCPLINT->localState->getWanderingHeroes();
-	listBox->reset();
+
+	listBox->resize(heroes.size());
+
+	for (size_t i = 0; i < heroes.size(); ++i)
+	{
+		auto item =  std::dynamic_pointer_cast<CHeroItem>(listBox->getItem(i));
+
+		if (!item)
+			continue;
+
+		if (item->hero == heroes[i])
+		{
+			item->update();
+		}
+		else
+		{
+			listBox->reset();
+			break;
+		}
+	}
+
 	if (LOCPLINT->localState->getCurrentHero())
 		select(LOCPLINT->localState->getCurrentHero());
 
@@ -364,8 +384,28 @@ void CTownList::updateElement(const CGTownInstance * town)
 
 void CTownList::updateWidget()
 {
-	listBox->resize(LOCPLINT->localState->getOwnedTowns().size());
+	auto & towns = LOCPLINT->localState->getOwnedTowns();
-	listBox->reset();
+
+	listBox->resize(towns.size());
+
+	for (size_t i = 0; i < towns.size(); ++i)
+	{
+		auto item =  std::dynamic_pointer_cast<CTownItem>(listBox->getItem(i));
+
+		if (!item)
+			continue;
+
+		if (item->town == towns[i])
+		{
+			item->update();
+		}
+		else
+		{
+			listBox->reset();
+			break;
+		}
+	}
+
 	if (LOCPLINT->localState->getCurrentTown())
 		select(LOCPLINT->localState->getCurrentTown());
 

client/gui/EventDispatcher.cpp

@@ -172,6 +172,15 @@ void EventDispatcher::dispatchClosePopup(const Point & position)
 
 void EventDispatcher::handleLeftButtonClick(const Point & position, bool isPressed)
 {
+	// WARNING: this approach is NOT SAFE
+	// 1) We allow (un)registering elements when list itself is being processed/iterated
+	// 2) To avoid iterator invalidation we create a copy of this list for processing
+	// HOWEVER it is completely possible (as in, actually happen, no just theory) to:
+	// 1) element gets unregistered and deleted from lclickable
+	// 2) element is completely deleted, as in - destructor called, memory freed
+	// 3) new element is created *with exactly same address(!)
+	// 4) new element is registered and code will incorrectly assume that this element is still registered
+	// POSSIBLE SOLUTION: make EventReceivers inherit from create_shared_from this and store weak_ptr's in lists
 	auto hlp = lclickable;
 	for(auto & i : hlp)
 	{