Fix connections not getting actually updated

Mods/vcmi/Content/config/rmg/symmetric/6lm10a.json

@@ -290,7 +290,7 @@
 			{ "a" : "16", "b" : "25", "guard" : 6000, "road" : "random"  },
 
 			{ "a" : "9", "b" : "10", "guard" : 6000, "road" : "true" },
-			{ "a" : "9", "b" : "21", "guard" : 6000, "road" : "random" },
+			{ "a" : "9", "b" : "21", "guard" : 6000, "road" : "false" },
 			{ "a" : "9", "b" : "23", "guard" : 6000, "road" : "false" },
 			{ "a" : "10", "b" : "22", "guard" : 6000, "road" : "random" },
 			{ "a" : "10", "b" : "23", "guard" : 6000, "road" : "random" },
@@ -298,7 +298,7 @@
 			{ "a" : "13", "b" : "24", "guard" : 6000, "road" : "random" },
 			{ "a" : "13", "b" : "14", "guard" : 6000, "road" : "true" },
 			{ "a" : "14", "b" : "23", "guard" : 6000, "road" : "false" },
-			{ "a" : "14", "b" : "25", "guard" : 6000, "road" : "random" },
+			{ "a" : "14", "b" : "25", "guard" : 6000, "road" : "false" },
 
 			{ "a" : "11", "b" : "21", "guard" : 6000, "road" : "true" },
 			{ "a" : "11", "b" : "24", "guard" : 6000, "road" : "random" },

lib/rmg/CRmgTemplate.cpp

@@ -302,6 +302,28 @@ std::vector<ZoneConnection> ZoneOptions::getConnections() const
 	return connectionDetails;
 }
 
+std::vector<ZoneConnection>& ZoneOptions::getConnectionsRef()
+{
+	return connectionDetails;
+}
+
+void ZoneOptions::setRoadOption(int connectionId, rmg::ERoadOption roadOption)
+{
+	for(auto & connection : connectionDetails)
+	{
+		if(connection.getId() == connectionId)
+		{
+			connection.setRoadOption(roadOption);
+			logGlobal->info("Set road option for connection %d between zones %d and %d to %s", 
+				connectionId, connection.getZoneA(), connection.getZoneB(), 
+				roadOption == rmg::ERoadOption::ROAD_TRUE ? "true" : 
+				roadOption == rmg::ERoadOption::ROAD_FALSE ? "false" : "random");
+			return;
+		}
+	}
+	logGlobal->warn("Failed to find connection with ID %d in zone %d", connectionId, id);
+}
+
 std::vector<TRmgTemplateZoneId> ZoneOptions::getConnectedZoneIds() const
 {
 	return connectedZoneIds;
@@ -481,12 +503,22 @@ rmg::ERoadOption ZoneConnection::getRoadOption() const
 {
 	return hasRoad;
 }
+
+void ZoneConnection::setRoadOption(rmg::ERoadOption roadOption)
+{
+	hasRoad = roadOption;
+}
 	
 bool operator==(const ZoneConnection & l, const ZoneConnection & r)
 {
 	return l.id == r.id;
 }
 
+bool operator<(const ZoneConnection & l, const ZoneConnection & r)
+{
+	return l.id < r.id;
+}
+
 void ZoneConnection::serializeJson(JsonSerializeFormat & handler)
 {
 	static const std::vector<std::string> connectionTypes =

lib/rmg/CRmgTemplate.h

@@ -105,10 +105,12 @@ public:
 	int getGuardStrength() const;
 	rmg::EConnectionType getConnectionType() const;
 	rmg::ERoadOption getRoadOption() const;
+	void setRoadOption(rmg::ERoadOption roadOption);
 
 	void serializeJson(JsonSerializeFormat & handler);
 	
 	friend bool operator==(const ZoneConnection &, const ZoneConnection &);
+	friend bool operator<(const ZoneConnection &, const ZoneConnection &);
 private:
 	int id;
 	TRmgTemplateZoneId zoneA;
@@ -185,8 +187,12 @@ public:
 
 	void addConnection(const ZoneConnection & connection);
 	std::vector<ZoneConnection> getConnections() const;
+	std::vector<ZoneConnection>& getConnectionsRef();
 	std::vector<TRmgTemplateZoneId> getConnectedZoneIds() const;
 
+	// Set road option for a specific connection by ID
+	void setRoadOption(int connectionId, rmg::ERoadOption roadOption);
+
 	void serializeJson(JsonSerializeFormat & handler);
 	
 	EMonsterStrength::EMonsterStrength monsterStrength;

lib/rmg/CZonePlacer.cpp

@@ -1008,127 +1008,104 @@ void CZonePlacer::assignZones(vstd::RNG * rand)
 void CZonePlacer::dropRandomRoads(vstd::RNG * rand)
 {
 	auto zones = map.getZones();
+	bool anyDropped;
 
-	//First, build a graph of road connections
-	std::map<TRmgTemplateZoneId, std::set<TRmgTemplateZoneId>> roadGraph;
-	std::vector<rmg::ZoneConnection> randomConnections;
-	std::vector<rmg::ZoneConnection> fixedConnections;
-
-	//Collect all road connections and build initial graph
-	for(const auto & zone : zones)
-	{
-		for(const auto & connection : zone.second->getConnections())
-		{
-			if(connection.getRoadOption() == rmg::ERoadOption::ROAD_TRUE)
-			{
-				roadGraph[connection.getZoneA()].insert(connection.getZoneB());
-				roadGraph[connection.getZoneB()].insert(connection.getZoneA());
-				fixedConnections.push_back(connection);
-			}
-			else if(connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM)
-			{
-				roadGraph[connection.getZoneA()].insert(connection.getZoneB());
-				roadGraph[connection.getZoneB()].insert(connection.getZoneA());
-				randomConnections.push_back(connection);
-			}
-		}
-	}
-
-	//Find all connected components in the initial graph
-	std::map<TRmgTemplateZoneId, int> zoneToComponent;
-	int numComponents = 0;
-
-	auto dfsComponent = [&](TRmgTemplateZoneId start, int component)
+	do
 	{
-		std::stack<TRmgTemplateZoneId> stack;
-		stack.push(start);
+		anyDropped = false;
+		std::map<TRmgTemplateZoneId, std::set<TRmgTemplateZoneId>> roadGraph;
+		std::set<rmg::ZoneConnection> randomConnections;
 
-		while(!stack.empty())
+		//Build graph and collect random connections
+		for(const auto & zone : zones)
 		{
-			auto current = stack.top();
-			stack.pop();
-
-			if(zoneToComponent.find(current) != zoneToComponent.end())
-				continue;
-
-			zoneToComponent[current] = component;
-
-			for(auto neighbor : roadGraph[current])
+			for(const auto & connection : zone.second->getConnections())
 			{
-				if(zoneToComponent.find(neighbor) == zoneToComponent.end())
+				if(connection.getRoadOption() == rmg::ERoadOption::ROAD_TRUE)
 				{
-					stack.push(neighbor);
+					roadGraph[connection.getZoneA()].insert(connection.getZoneB());
+					roadGraph[connection.getZoneB()].insert(connection.getZoneA());
 				}
+				else if(connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM)
+				{
+					roadGraph[connection.getZoneA()].insert(connection.getZoneB());
+					roadGraph[connection.getZoneB()].insert(connection.getZoneA());
+					randomConnections.insert(connection);
+				}
+				// ROAD_FALSE connections are ignored
 			}
 		}
-	};
+		logGlobal->info("Remaining random connections: %d", randomConnections.size());
 
-	//Find all components
-	for(const auto & zone : zones)
-	{
-		if(zoneToComponent.find(zone.first) == zoneToComponent.end() && !roadGraph[zone.first].empty())
-		{
-			dfsComponent(zone.first, numComponents++);
-		}
-	}
+		if(randomConnections.empty())
+			break;
 
-	//Process each component separately
-	for(int component = 0; component < numComponents; component++)
-	{
-		//Get random connections for this component
-		std::vector<rmg::ZoneConnection> componentRandomConnections;
-		for(const auto & conn : randomConnections)
+		//Convert to vector for shuffling
+		std::vector<rmg::ZoneConnection> shuffledConnections(randomConnections.begin(), randomConnections.end());
+		RandomGeneratorUtil::randomShuffle(shuffledConnections, *rand);
+
+		//Try each random connection in shuffled order
+		for(const auto & conn : shuffledConnections)
 		{
-			if(zoneToComponent[conn.getZoneA()] == component)
+			auto zoneA = conn.getZoneA();
+			auto zoneB = conn.getZoneB();
+
+			//Check if either zone would become isolated by removing this connection
+			if(roadGraph[zoneA].size() <= 1 || roadGraph[zoneB].size() <= 1)
 			{
-				componentRandomConnections.push_back(conn);
+				//Can't remove this connection as it would isolate a zone
+				continue;
 			}
-		}
-
-		//Shuffle random connections
-		RandomGeneratorUtil::randomShuffle(componentRandomConnections, *rand);
 
-		//Try removing each random connection
-		for(const auto & conn : componentRandomConnections)
-		{
 			//Temporarily remove this connection
-			roadGraph[conn.getZoneA()].erase(conn.getZoneB());
-			roadGraph[conn.getZoneB()].erase(conn.getZoneA());
+			roadGraph[zoneA].erase(zoneB);
+			roadGraph[zoneB].erase(zoneA);
 
-			//Check if graph remains connected
+			//Check if graph remains connected as a whole
 			bool canRemove = true;
 			std::set<TRmgTemplateZoneId> visited;
 
-			//Start DFS from any zone in this component
-			auto startZone = conn.getZoneA();
-			std::stack<TRmgTemplateZoneId> stack;
-			stack.push(startZone);
+			// Get all zones that have road connections
+			std::set<TRmgTemplateZoneId> zonesWithRoads;
+			for(const auto & entry : roadGraph)
+			{
+				if(!entry.second.empty())
+				{
+					zonesWithRoads.insert(entry.first);
+				}
+			}
 
-			while(!stack.empty())
+			if(!zonesWithRoads.empty())
 			{
-				auto current = stack.top();
-				stack.pop();
+				//Start DFS from any zone that has roads
+				TRmgTemplateZoneId startZone = *zonesWithRoads.begin();
+				
+				std::stack<TRmgTemplateZoneId> stack;
+				stack.push(startZone);
 
-				if(visited.find(current) != visited.end())
-					continue;
+				while(!stack.empty())
+				{
+					auto current = stack.top();
+					stack.pop();
 
-				visited.insert(current);
+					if(visited.find(current) != visited.end())
+						continue;
 
-				for(auto neighbor : roadGraph[current])
-				{
-					if(visited.find(neighbor) == visited.end())
+					visited.insert(current);
+
+					for(auto neighbor : roadGraph[current])
 					{
-						stack.push(neighbor);
+						if(visited.find(neighbor) == visited.end())
+						{
+							stack.push(neighbor);
+						}
 					}
 				}
-			}
 
-			//Check if all zones in this component are still reachable
-			for(const auto & zone : zones)
-			{
-				if(zoneToComponent[zone.first] == component && !roadGraph[zone.first].empty())
+				//Check if all zones with roads are still reachable
+				for(auto zoneId : zonesWithRoads)
 				{
-					if(visited.find(zone.first) == visited.end())
+					if(visited.find(zoneId) == visited.end())
 					{
 						canRemove = false;
 						break;
@@ -1138,24 +1115,44 @@ void CZonePlacer::dropRandomRoads(vstd::RNG * rand)
 
 			if(!canRemove)
 			{
-				//Restore connection if removing it would break connectivity
-				roadGraph[conn.getZoneA()].insert(conn.getZoneB());
-				roadGraph[conn.getZoneB()].insert(conn.getZoneA());
+				//Restore connection and try next one
+				roadGraph[zoneA].insert(zoneB);
+				roadGraph[zoneB].insert(zoneA);
+				continue;
 			}
-			else
+
+			//Found a connection we can remove - update in both zones that contain it
+			auto & zonePtr = zones[zoneA];
+			zonePtr->setRoadOption(conn.getId(), rmg::ERoadOption::ROAD_FALSE);
+
+			auto & otherZonePtr = zones[zoneB];
+			otherZonePtr->setRoadOption(conn.getId(), rmg::ERoadOption::ROAD_FALSE);
+
+			logGlobal->info("Dropped random road between %d and %d", zoneA, zoneB);
+			anyDropped = true;
+			break; //Exit loop and rebuild graph
+		}
+	} while(anyDropped);
+
+	// Use a set to track processed connection IDs to avoid duplicates
+	std::set<int> processedConnectionIds;
+	
+	// Process each zone's connections
+	for(auto & zonePtr : zones)
+	{
+		for(auto & connection : zonePtr.second->getConnections())
+		{			
+			if(connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM)
 			{
-				//Mark this connection as having no road
-				for(auto & zonePtr : zones)
-				{
-					for(auto & connection : zonePtr.second->getConnections())
-					{
-						if(connection.getId() == conn.getId())
-						{
-							const_cast<rmg::ZoneConnection&>(connection) = conn; //FIXME: avoid const_cast
-							break;
-						}
-					}
-				}
+				auto id = connection.getId();
+				// Only process each connection once
+				if(vstd::contains(processedConnectionIds, id))
+					continue;
+
+				processedConnectionIds.insert(id);
+				
+				// Use the new setRoadOption method
+				zonePtr.second->setRoadOption(id, rmg::ERoadOption::ROAD_TRUE);
 			}
 		}
 	}

lib/rmg/modificators/ConnectionsPlacer.cpp

@@ -110,14 +110,10 @@ void ConnectionsPlacer::init()
 	POSTFUNCTION(RoadPlacer);
 	POSTFUNCTION(ObjectManager);
 	
-	auto id = zone.getId();
-	for(auto c : map.getMapGenOptions().getMapTemplate()->getConnectedZoneIds())
+	// FIXME: Use zones modified by CZonePlacer
+	for (auto c : zone.getConnections())
 	{
-		// Only consider connected zones
-		if (c.getZoneA() == id || c.getZoneB() == id)
-		{
-			addConnection(c);
-		}
+		addConnection(c);
 	}
 }
 
@@ -477,6 +473,10 @@ void ConnectionsPlacer::collectNeighbourZones()
 
 bool ConnectionsPlacer::shouldGenerateRoad(const rmg::ZoneConnection& connection) const
 {
+	if (connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM)
+		logGlobal->error("Random road between zones %d and %d", connection.getZoneA(), connection.getZoneB());
+	else
+		logGlobal->info("Should generate road between zones %d and %d: %d", connection.getZoneA(), connection.getZoneB(), connection.getRoadOption() == rmg::ERoadOption::ROAD_TRUE);
 	return connection.getRoadOption() == rmg::ERoadOption::ROAD_TRUE;
 }