diff --git a/client/adventureMap/CList.cpp b/client/adventureMap/CList.cpp index 23c325ea9..e53968d9f 100644 --- a/client/adventureMap/CList.cpp +++ b/client/adventureMap/CList.cpp @@ -287,8 +287,28 @@ void CHeroList::updateElement(const CGHeroInstance * hero) void CHeroList::updateWidget() { - listBox->resize(LOCPLINT->localState->getWanderingHeroes().size()); - listBox->reset(); + auto & heroes = LOCPLINT->localState->getWanderingHeroes(); + + listBox->resize(heroes.size()); + + for (size_t i = 0; i < heroes.size(); ++i) + { + auto item = std::dynamic_pointer_cast(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()); - listBox->reset(); + auto & towns = LOCPLINT->localState->getOwnedTowns(); + + listBox->resize(towns.size()); + + for (size_t i = 0; i < towns.size(); ++i) + { + auto item = std::dynamic_pointer_cast(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()); diff --git a/client/gui/EventDispatcher.cpp b/client/gui/EventDispatcher.cpp index 290cf1ee6..c09d95826 100644 --- a/client/gui/EventDispatcher.cpp +++ b/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) {