Final implementation

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

@@ -261,7 +261,7 @@
 			{ "a" : "1", "b" : "21", "guard" : 3000, "road" : "true" },
 
 			{ "a" : "2", "b" : "10", "guard" : 6000, "road" : "random" },
-			{ "a" : "2", "b" : "22", "guard" : 3000, "road" : "true" },
+			{ "a" : "2", "b" : "22", "guard" : 3000, "road" : "random" },
 			{ "a" : "2", "b" : "22", "guard" : 3000, "road" : "random" },
 
 			{ "a" : "3", "b" : "21", "guard" : 3000, "road" : "true" },
@@ -272,7 +272,7 @@
 			{ "a" : "4", "b" : "23", "guard" : 3000, "road" : "false" },
 			{ "a" : "4", "b" : "25", "guard" : 3000, "road" : "true" },
 
-			{ "a" : "5", "b" : "13", "guard" : 6000, "road" : "true" },
+			{ "a" : "5", "b" : "13", "guard" : 6000, "road" : "random " },
 			{ "a" : "5", "b" : "24", "guard" : 3000, "road" : "random" },
 			{ "a" : "5", "b" : "24", "guard" : 3000, "road" : "random" },
 

lib/rmg/CRoadRandomizer.cpp

@@ -44,7 +44,7 @@ void unionSets(std::map<TRmgTemplateZoneId, TRmgTemplateZoneId> & parent, TRmgTe
 Random road generation requirements:
 - Every town should be connected via road
 - There should be exactly one road betwen any two towns (connected MST)
-	- This excludes cases when there are multiple road connetions betwween two zones
+	- This excludes cases when there are multiple obligatory road connections betwween two zones
 - Road cannot end in a zone without town
 - Wide connections should have no road
 */
@@ -102,92 +102,19 @@ void CRoadRandomizer::dropRandomRoads(vstd::RNG * rand)
 		return;
 	}
 	
-	// Track direct connections between zones (both TRUE and RANDOM)
-	std::map<TRmgTemplateZoneId, std::map<TRmgTemplateZoneId, bool>> directConnections;
-	
-	// First pass: find all TRUE connections
-	for(auto & zonePtr : zones)
-	{
-		for(auto & connection : zonePtr.second->getConnections())
-		{
-			auto zoneA = connection.getZoneA();
-			auto zoneB = connection.getZoneB();
-			
-			if(connection.getRoadOption() == rmg::ERoadOption::ROAD_TRUE)
-			{
-				// Mark that these zones are directly connected by a TRUE road
-				directConnections[zoneA][zoneB] = true;
-				directConnections[zoneB][zoneA] = true;
-			}
-		}
-	}
-	
-	// Track all available connections in the template (for graph connectivity analysis)
-	std::map<TRmgTemplateZoneId, std::set<TRmgTemplateZoneId>> allConnections;
-	std::map<std::pair<TRmgTemplateZoneId, TRmgTemplateZoneId>, int> connectionIds;
-	
-	// Build adjacency list for available connections and track connection IDs
-	for(auto & zonePtr : zones)
-	{
-		for(auto & connection : zonePtr.second->getConnections())
-		{
-			auto zoneA = connection.getZoneA();
-			auto zoneB = connection.getZoneB();
-			
-			if(connection.getRoadOption() == rmg::ERoadOption::ROAD_TRUE ||
-			   connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM)
-			{
-				// Only include TRUE and RANDOM roads in connectivity analysis
-				allConnections[zoneA].insert(zoneB);
-				allConnections[zoneB].insert(zoneA);
-				
-				// Track connection ID for later use
-				connectionIds[{std::min(zoneA, zoneB), std::max(zoneA, zoneB)}] = connection.getId();
-			}
-		}
-	}
-	
-	// Check if there's a path between all town zones using all available connections
-	// This is to verify if global connectivity is even possible
-	std::map<TRmgTemplateZoneId, std::set<TRmgTemplateZoneId>> reachableTowns;
-	
-	for(auto startTown : zonesWithTowns)
-	{
-		std::queue<TRmgTemplateZoneId> q;
-		std::set<TRmgTemplateZoneId> visited;
-		
-		q.push(startTown);
-		visited.insert(startTown);
-		
-		while(!q.empty())
-		{
-			auto current = q.front();
-			q.pop();
-			
-			for(auto neighbor : allConnections[current])
-			{
-				if(!vstd::contains(visited, neighbor))
-				{
-					visited.insert(neighbor);
-					q.push(neighbor);
-					
-					if(vstd::contains(zonesWithTowns, neighbor))
-					{
-						reachableTowns[startTown].insert(neighbor);
-					}
-				}
-			}
-		}
-	}
-	
-	// Initialize Union-Find for MST tracking
+	// Initialize Union-Find for all zones to track connectivity
 	std::map<TRmgTemplateZoneId, TRmgTemplateZoneId> parent;
-	for(auto townZone : zonesWithTowns)
+	// Track if a component (represented by its root) contains a town
+	std::map<TRmgTemplateZoneId, bool> componentHasTown;
+
+	for(const auto & zone : zones)
 	{
-		parent[townZone] = townZone;
+		auto zoneId = zone.first;
+		parent[zoneId] = zoneId;
+		componentHasTown[zoneId] = vstd::contains(zonesWithTowns, zoneId);
 	}
-	
-	// Add all TRUE roads connecting town zones to MST
+
+	// Process all connections that are already set to TRUE
 	for(auto & zonePtr : zones)
 	{
 		for(auto & connection : zonePtr.second->getConnections())
@@ -196,368 +123,80 @@ void CRoadRandomizer::dropRandomRoads(vstd::RNG * rand)
 			{
 				auto zoneA = connection.getZoneA();
 				auto zoneB = connection.getZoneB();
-				
-				// If both zones have towns, add to MST
-				if(vstd::contains(zonesWithTowns, zoneA) && vstd::contains(zonesWithTowns, zoneB))
-				{
-					unionSets(parent, zoneA, zoneB);
-				}
-			}
-		}
-	}
-	
-	// Process all paths through true roads (BFS)
-	for(auto townZone : zonesWithTowns)
-	{
-		std::queue<TRmgTemplateZoneId> q;
-		std::set<TRmgTemplateZoneId> visited;
-		
-		q.push(townZone);
-		visited.insert(townZone);
-		
-		while(!q.empty())
-		{
-			auto current = q.front();
-			q.pop();
-			
-			for(auto & otherZone : directConnections[current])
-			{
-				if(otherZone.second && !vstd::contains(visited, otherZone.first))
-				{
-					visited.insert(otherZone.first);
-					q.push(otherZone.first);
-					
-					// If this is another town, update MST
-					if(vstd::contains(zonesWithTowns, otherZone.first))
-					{
-						unionSets(parent, townZone, otherZone.first);
-					}
-				}
-			}
-		}
-	}
-	
-	// Process RANDOM connections 
-	// First, ensure each town has at least one connection
-	for(auto townZone : zonesWithTowns)
-	{
-		// Skip if already has a TRUE road
-		if(!directConnections[townZone].empty())
-			continue;
-			
-		// Find a random connection to upgrade to TRUE
-		for(auto & zonePtr : zones)
-		{
-			for(auto & connection : zonePtr.second->getConnections())
-			{
-				if(connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM &&
-				   (connection.getZoneA() == townZone || connection.getZoneB() == townZone))
+				auto rootA = findSet(parent, zoneA);
+				auto rootB = findSet(parent, zoneB);
+
+				if(rootA != rootB)
 				{
-					auto zoneA = connection.getZoneA();
-					auto zoneB = connection.getZoneB();
-					
-					// Don't upgrade if these zones are already directly connected by a TRUE road
-					if(vstd::contains(directConnections[zoneA], zoneB) && directConnections[zoneA][zoneB])
-						continue;
-						
-					// Upgrade to TRUE
-					setRoadOptionForConnection(connection.getId(), rmg::ERoadOption::ROAD_TRUE);
-					directConnections[zoneA][zoneB] = true;
-					directConnections[zoneB][zoneA] = true;
-					
-					auto otherZone = (zoneA == townZone) ? zoneB : zoneA;
-					logGlobal->info("Setting RANDOM road to TRUE for connection %d to ensure town zone %d has at least one connection (to zone %d)", 
-									connection.getId(), townZone, otherZone);
-					
-					break;
+					bool hasTown = componentHasTown[rootA] || componentHasTown[rootB];
+					parent[rootA] = rootB;
+					componentHasTown[rootB] = hasTown;
 				}
 			}
-			
-			// Stop if we've found a connection
-			if(!directConnections[townZone].empty())
-				break;
 		}
 	}
-	
-	// Process remaining RANDOM roads - prioritize town connectivity
-	// First collect all RANDOM roads
-	std::vector<std::pair<int, std::pair<TRmgTemplateZoneId, TRmgTemplateZoneId>>> randomRoads;
-	
+
+	// Collect all RANDOM connections to be processed
+	std::vector<rmg::ZoneConnection> randomRoads;
+	std::map<int, bool> processedConnectionIds;
+
 	for(auto & zonePtr : zones)
 	{
 		for(auto & connection : zonePtr.second->getConnections())
 		{
 			if(connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM)
 			{
-				auto id = connection.getId();
-				auto zoneA = connection.getZoneA();
-				auto zoneB = connection.getZoneB();
-				
-				// Skip if these zones are already directly connected by a TRUE road
-				if(vstd::contains(directConnections[zoneA], zoneB) && directConnections[zoneA][zoneB])
+				// Ensure we only add each connection once
+				if(processedConnectionIds.find(connection.getId()) == processedConnectionIds.end())
 				{
-					setRoadOptionForConnection(id, rmg::ERoadOption::ROAD_FALSE);
-					logGlobal->info("Setting RANDOM road to FALSE for connection %d - duplicate of TRUE road between zones %d and %d", 
-									id, zoneA, zoneB);
-					continue;
+					randomRoads.push_back(connection);
+					processedConnectionIds[connection.getId()] = true;
 				}
-				
-				randomRoads.push_back(std::make_pair(id, std::make_pair(zoneA, zoneB)));
 			}
 		}
 	}
-	
+
 	RandomGeneratorUtil::randomShuffle(randomRoads, *rand);
-	
-	// Process random roads - first connect town zones
-	for(auto& road : randomRoads)
+
+	// Process random roads using Kruskal's-like algorithm to connect components
+	for(const auto& connection : randomRoads)
 	{
-		auto id = road.first;
-		auto zoneA = road.second.first;
-		auto zoneB = road.second.second;
-		
-		bool setToTrue = false;
-		
-		// If both zones have towns, check if they're already connected in the MST
-		if(vstd::contains(zonesWithTowns, zoneA) && vstd::contains(zonesWithTowns, zoneB))
-		{
-			if(findSet(parent, zoneA) != findSet(parent, zoneB))
-			{
-				// Not connected, add this road to MST
-				unionSets(parent, zoneA, zoneB);
-				setToTrue = true;
-				
-				logGlobal->info("Setting RANDOM road to TRUE for connection %d between town zones %d and %d", 
-								id, zoneA, zoneB);
-			}
-		}
-		// If one zone has a town and one doesn't
-		else if(vstd::contains(zonesWithTowns, zoneA) || vstd::contains(zonesWithTowns, zoneB))
+		auto zoneA = connection.getZoneA();
+		auto zoneB = connection.getZoneB();
+		auto rootA = findSet(parent, zoneA);
+		auto rootB = findSet(parent, zoneB);
+
+		bool roadSet = false;
+
+		// Only build roads if they connect different components
+		if(rootA != rootB)
 		{
-			TRmgTemplateZoneId townZone = vstd::contains(zonesWithTowns, zoneA) ? zoneA : zoneB;
-			TRmgTemplateZoneId nonTownZone = vstd::contains(zonesWithTowns, zoneA) ? zoneB : zoneA;
-			
-			// Check if this town already has at least one TRUE connection
-			if(directConnections[townZone].empty())
-			{
-				setToTrue = true;
-				logGlobal->info("Setting RANDOM road to TRUE for connection %d - only connection for town zone %d", 
-								id, townZone);
-			}
-			else
+			bool townInA = componentHasTown[rootA];
+			bool townInB = componentHasTown[rootB];
+
+			// Connect components if at least one of them contains a town.
+			// This ensures we connect all town components and extend them,
+			// but never connect two non-town components together.
+			if(townInA || townInB)
 			{
-				// See if this non-town zone connects to another town zone
-				// This could be a potential bridge zone to connect towns
-				bool connectsToOtherTown = false;
-				TRmgTemplateZoneId otherTownZone = 0;
-				
-				for(auto connectedZone : allConnections[nonTownZone])
-				{
-					if(vstd::contains(zonesWithTowns, connectedZone) && connectedZone != townZone)
-					{
-						otherTownZone = connectedZone;
-						connectsToOtherTown = true;
-						break;
-					}
-				}
+				logGlobal->info("Setting RANDOM road to TRUE for connection %d between zones %d and %d to connect town components.", connection.getId(), zoneA, zoneB);
+				setRoadOptionForConnection(connection.getId(), rmg::ERoadOption::ROAD_TRUE);
 				
-				if(connectsToOtherTown && findSet(parent, townZone) != findSet(parent, otherTownZone))
-				{
-					// This non-town zone can help connect two town zones that are not yet connected
-					setToTrue = true;
-					logGlobal->info("Setting RANDOM road to TRUE for connection %d - bridge through non-town zone %d to connect towns %d and %d", 
-									id, nonTownZone, townZone, otherTownZone);
-				}
-			}
-		}
-		
-		// Update all zones with this connection
-		setRoadOptionForConnection(id, setToTrue ? rmg::ERoadOption::ROAD_TRUE : rmg::ERoadOption::ROAD_FALSE);
-		
-		if(setToTrue)
-		{
-			directConnections[zoneA][zoneB] = true;
-			directConnections[zoneB][zoneA] = true;
-		}
-	}
-	
-	// Check if we have a connected graph for town zones after initial MST
-	std::map<TRmgTemplateZoneId, std::set<TRmgTemplateZoneId>> connectedComponents;
-	std::set<TRmgTemplateZoneId> processedTowns;
-	
-	for(auto townZone : zonesWithTowns)
-	{
-		if(vstd::contains(processedTowns, townZone))
-			continue;
-			
-		std::set<TRmgTemplateZoneId> component;
-		for(auto otherTown : zonesWithTowns)
-		{
-			if(findSet(parent, townZone) == findSet(parent, otherTown))
-			{
-				component.insert(otherTown);
-				processedTowns.insert(otherTown);
+				// Union the sets
+				bool hasTown = townInA || townInB;
+				parent[rootA] = rootB;
+				componentHasTown[rootB] = hasTown;
+				roadSet = true;
 			}
 		}
-		
-		if(!component.empty())
-			connectedComponents[townZone] = component;
-	}
-	
-	// If we have more than one component, try to connect them if possible
-	if(connectedComponents.size() > 1)
-	{
-		logGlobal->warn("Found %d disconnected town components, trying to connect them", connectedComponents.size());
-		
-		// Create a list of components
-		std::vector<std::set<TRmgTemplateZoneId>> components;
-		for(auto & component : connectedComponents)
-		{
-			components.push_back(component.second);
-		}
-		
-		// For each pair of components, try to find a path between them
-		for(size_t i = 0; i < components.size() - 1; i++)
-		{
-			bool foundBridge = false;
-			
-			for(size_t j = i + 1; j < components.size() && !foundBridge; j++)
-			{
-				// Try to find a path between any two towns in different components
-				for(auto townA : components[i])
-				{
-					if(foundBridge) break;
-					
-					for(auto townB : components[j])
-					{
-						// Check if there's a path between townA and townB in the original template
-						if(vstd::contains(reachableTowns[townA], townB))
-						{
-							// There's a path, now find the specific path to enable roads on
-							std::queue<TRmgTemplateZoneId> q;
-							std::map<TRmgTemplateZoneId, TRmgTemplateZoneId> prev;
-							
-							q.push(townA);
-							prev[townA] = 0; // Mark as visited with no predecessor
-							
-							bool found = false;
-							while(!q.empty() && !found)
-							{
-								auto current = q.front();
-								q.pop();
-								
-								for(auto next : allConnections[current])
-								{
-									if(!vstd::contains(prev, next))
-									{
-										prev[next] = current;
-										q.push(next);
-										
-										if(next == townB)
-										{
-											found = true;
-											break;
-										}
-									}
-								}
-							}
-							
-							// Now reconstruct the path and set all roads on this path to TRUE
-							if(found)
-							{
-								std::vector<TRmgTemplateZoneId> path;
-								TRmgTemplateZoneId current = townB;
-								
-								while(current != townA)
-								{
-									path.push_back(current);
-									current = prev[current];
-								}
-								path.push_back(townA);
-								
-								// Reverse to get path from townA to townB
-								std::reverse(path.begin(), path.end());
-								
-								logGlobal->info("Found path between town zones %d and %d, enabling all roads on this path", townA, townB);
-								
-								// Enable all roads on this path
-								for(size_t k = 0; k < path.size() - 1; k++)
-								{
-									auto zoneA = path[k];
-									auto zoneB = path[k+1];
-									
-									auto minZone = std::min(zoneA, zoneB);
-									auto maxZone = std::max(zoneA, zoneB);
-									
-									if(vstd::contains(connectionIds, std::make_pair(minZone, maxZone)))
-									{
-										auto connectionId = connectionIds[std::make_pair(minZone, maxZone)];
-										
-										// Enable this road if it's not already TRUE
-										for(auto & zonePtr : zones)
-										{
-											for(auto & connection : zonePtr.second->getConnections())
-											{
-												if(connection.getId() == connectionId && 
-												   connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM)
-												{
-													setRoadOptionForConnection(connectionId, rmg::ERoadOption::ROAD_TRUE);
-													directConnections[zoneA][zoneB] = true;
-													directConnections[zoneB][zoneA] = true;
-													
-													logGlobal->info("Setting RANDOM road to TRUE for connection %d to connect components - part of path between towns %d and %d", 
-														connectionId, townA, townB);
-													
-													break;
-												}
-											}
-										}
-									}
-								}
-								
-								// Update Union-Find to merge components
-								unionSets(parent, townA, townB);
-								foundBridge = true;
-								break;
-							}
-						}
-					}
-				}
-			}
-			
-			if(!foundBridge)
-			{
-				logGlobal->warn("Could not find a path between component with towns [%s] and other components", 
-					[&components, i]() {
-						std::string result;
-						for(auto town : components[i])
-						{
-							if(!result.empty()) result += ", ";
-							result += std::to_string(town);
-						}
-						return result;
-					}().c_str());
-			}
-		}
-	}
-	
-	// Final check for connectivity between town zones
-	std::set<TRmgTemplateZoneId> connectedTowns;
-	if(!zonesWithTowns.empty())
-	{
-		auto firstTown = *zonesWithTowns.begin();
-		for(auto town : zonesWithTowns)
+
+		if(!roadSet)
 		{
-			if(findSet(parent, firstTown) == findSet(parent, town))
-			{
-				connectedTowns.insert(town);
-			}
+			// This road was not chosen, either because it creates a cycle or connects two non-town areas
+			setRoadOptionForConnection(connection.getId(), rmg::ERoadOption::ROAD_FALSE);
 		}
 	}
-	
-	logGlobal->info("Final town connectivity: %d connected out of %d total town zones", 
-					connectedTowns.size(), zonesWithTowns.size());
-	
+
 	logGlobal->info("Finished road generation - created minimal spanning tree connecting all towns");
 }