Fix random map generation

lib/gameState/CGameState.cpp

@@ -291,13 +291,13 @@ void CGameState::initNewGame(const IMapService * mapService, bool allowSavingRan
 		CMapGenerator mapGenerator(*scenarioOps->mapGenOptions, cb, getRandomGenerator().nextInt());
 		progressTracking.include(mapGenerator);
 
-		std::unique_ptr<CMap> randomMap = mapGenerator.generate();
+		map = mapGenerator.generate();
 		progressTracking.exclude(mapGenerator);
 
 		// Update starting options
-		for(int i = 0; i < randomMap->players.size(); ++i)
+		for(int i = 0; i < map->players.size(); ++i)
 		{
-			const auto & playerInfo = randomMap->players[i];
+			const auto & playerInfo = map->players[i];
 			if(playerInfo.canAnyonePlay())
 			{
 				PlayerSettings & playerSettings = scenarioOps->playerInfos[PlayerColor(i)];
@@ -330,9 +330,9 @@ void CGameState::initNewGame(const IMapService * mapService, bool allowSavingRan
 				const std::string fileName = boost::str(boost::format("%s_%s.vmap") % dt % templateName );
 				const auto fullPath = path / fileName;
 
-				randomMap->name.appendRawString(boost::str(boost::format(" %s") % dt));
+				map->name.appendRawString(boost::str(boost::format(" %s") % dt));
 
-				mapService->saveMap(randomMap, fullPath);
+				mapService->saveMap(map, fullPath);
 
 				logGlobal->info("Random map has been saved to:");
 				logGlobal->info(fullPath.string());
@@ -343,7 +343,6 @@ void CGameState::initNewGame(const IMapService * mapService, bool allowSavingRan
 			}
 		}
 
-		map = std::move(randomMap);
 
 		logGlobal->info("Generated random map in %i ms.", sw.getDiff());
 	}
@@ -631,9 +630,11 @@ void CGameState::initHeroes()
 			auto newHeroPtr = std::make_shared<CGHeroInstance>(cb);
 			newHeroPtr->subID = htype.getNum();
 			map->addToHeroPool(newHeroPtr);
+			map->generateUniqueInstanceName(newHeroPtr.get());
 			heroInPool = newHeroPtr.get();
 		}
 		heroInPool->initHero(getRandomGenerator());
+		heroesPool->addHeroToPool(htype);
 	}
 
 	for(auto & elem : map->disposedHeroes)

lib/mapping/CMap.cpp

@@ -517,7 +517,11 @@ void CMap::addNewObject(std::shared_ptr<CGObjectInstance> obj)
 	if (vstd::contains(instanceNames, obj->instanceName))
 		throw std::runtime_error("Object instance name duplicated: "+obj->instanceName);
 
-	objects.emplace_back(obj);
+	if (obj->id == ObjectInstanceID(objects.size()))
+		objects.emplace_back(obj);
+	else
+		objects[obj->id.getNum()] = obj;
+
 	instanceNames[obj->instanceName] = obj;
 	addBlockVisTiles(obj.get());
 
@@ -540,17 +544,34 @@ std::shared_ptr<CGObjectInstance> CMap::removeObject(ObjectInstanceID oldObject)
 
 	removeBlockVisTiles(obj.get());
 	instanceNames.erase(obj->instanceName);
+	obj->afterRemoveFromMap(this);
 
 	//update indices
 
 	auto iter = std::next(objects.begin(), obj->id.getNum());
 	iter = objects.erase(iter);
-	for(int i = obj->id.getNum(); iter != objects.end(); ++i, ++iter)
-	{
-		(*iter)->id = ObjectInstanceID(i);
-	}
 
-	obj->afterRemoveFromMap(this);
+	for(int i = obj->id.getNum(); iter != objects.end(); ++i, ++iter)
+		(*iter)->id = ObjectInstanceID(i);
+
+	for (auto & town : towns)
+		if (town.getNum() >= obj->id)
+			town = ObjectInstanceID(town.getNum()-1);
+
+	for (auto & hero : heroesOnMap)
+		if (hero.getNum() >= obj->id)
+			hero = ObjectInstanceID(hero.getNum()-1);
+
+	for(auto tile = terrain.origin(); tile < (terrain.origin() + terrain.num_elements()); ++tile)
+	{
+		for (auto & objectID : tile->blockingObjects)
+			if (objectID.getNum() >= obj->id)
+				objectID = ObjectInstanceID(objectID.getNum()-1);
+
+		for (auto & objectID : tile->visitableObjects)
+			if (objectID.getNum() >= obj->id)
+				objectID = ObjectInstanceID(objectID.getNum()-1);
+	}
 
 	//TODO: Clean artifact instances (mostly worn by hero?) and quests related to this object
 	//This causes crash with undo/redo in editor
@@ -718,6 +739,8 @@ void CMap::reindexObjects()
 {
 	// Only reindex at editor / RMG operations
 
+	auto oldIndex = objects;
+
 	std::sort(objects.begin(), objects.end(), [](const auto & lhs, const auto & rhs)
 	{
 		// Obstacles first, then visitable, at the end - removable
@@ -743,8 +766,21 @@ void CMap::reindexObjects()
 
 	// instanceNames don't change
 	for (size_t i = 0; i < objects.size(); ++i)
-	{
 		objects[i]->id = ObjectInstanceID(i);
+
+	for (auto & town : towns)
+		town = oldIndex.at(town.getNum())->id;
+
+	for (auto & hero : heroesOnMap)
+		hero = oldIndex.at(hero.getNum())->id;
+
+	for(auto tile = terrain.origin(); tile < (terrain.origin() + terrain.num_elements()); ++tile)
+	{
+		for (auto & objectID : tile->blockingObjects)
+			objectID = oldIndex.at(objectID.getNum())->id;
+
+		for (auto & objectID : tile->visitableObjects)
+			objectID = oldIndex.at(objectID.getNum())->id;
 	}
 }
 

lib/networkPacks/NetPacksLib.cpp

@@ -1433,16 +1433,11 @@ void HeroRecruited::applyGs(CGameState *gs)
 	h->pos = tile;
 	h->updateAppearance();
 
-	assert(h->id == ObjectInstanceID());
-	if(h->id == ObjectInstanceID())
-	{
-		gs->getMap().addNewObject(h);
-	}
-	else
-		gs->getMap().replaceObject(h->id, h);
+	assert(h->id.hasValue());
+	gs->getMap().addNewObject(h);
 
 	p->addOwnedObject(h.get());
-	h->attachTo(*p);
+	h->attachToBonusSystem(gs);
 
 	if(t)
 		t->setVisitingHero(h.get());

server/processors/HeroPoolProcessor.cpp

@@ -109,7 +109,9 @@ void HeroPoolProcessor::selectNewHeroForSlot(const PlayerColor & color, TavernHe
 	sah.slotID = slot;
 	sah.replenishPoints = true;
 
-	CGHeroInstance *newHero = (nextHero == HeroTypeID::NONE) ? pickHeroFor(needNativeHero, color) : gameHandler->gameState()->heroesPool->unusedHeroesFromPool()[nextHero];
+	CGHeroInstance *newHero = nextHero.hasValue()?
+		gameHandler->gameState()->heroesPool->unusedHeroesFromPool()[nextHero]:
+		pickHeroFor(needNativeHero, color);
 
 	if (newHero)
 	{