From 061b39864cbbf391a2bc7ecc056d51ff56e0679c Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 11:48:09 -0700 Subject: [PATCH] Allow several pipe groups to share a centroid Group.Location is the rounded centroid of the group's pumpjack centers: Location = new Location( (int)Math.Round(_sumX / _entities.Count, 0), (int)Math.Round(_sumY / _entities.Count, 0)); That is a derived value, and nothing makes it unique. Two groups can round to the same tile. DelaunayTriangulation used it as a dictionary key through ToDictionary and Add, both of which throw ArgumentException on a duplicate, so a collision took the whole plan down with "An item with the same key has already been added". Key by centroid to a list of groups instead. Where the triangulation used to map a line endpoint to one group, it now maps to every group at that point. Groups sharing the popped group's own centroid need handling separately. The triangulation sees a single point for all of them, so they can never appear as the far end of a line, yet they are the closest partners there are. They are added directly, after the line pass. This leaves every existing plan byte for byte identical, which is the point of doing it on its own. When no two groups share a centroid each bucket holds exactly one group, the line pass adds the same partners in the same order, and the direct pass adds nothing because the only group at the popped group's centroid is itself. Measured: the full suite passes 4299 of 4299 with zero changed Verify snapshots. No test fails without this change, and that is worth stating plainly rather than hiding. The collision does not occur on any of the 1147 big-list blueprints as they are grouped today, and a search over roughly 8000 synthetic layouts did not produce one either. The only known reproduction is big-list blueprint 904 once the pumpjack terminal offsets are corrected for Factorio 2.1, which is the next change. Verified against it: with those offsets applied on top of this commit, blueprint 904 plans instead of throwing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01P9FADuTnjE7SFEQWnpNhfc --- .../OilField/Steps/AddPipes.1.FBE.cs | 76 +++++++++++++++-- src/lua/FactorioTools/AddPipes_1_FBE.lua | 81 ++++++++++++++++--- 2 files changed, 142 insertions(+), 15 deletions(-) diff --git a/src/FactorioTools/OilField/Steps/AddPipes.1.FBE.cs b/src/FactorioTools/OilField/Steps/AddPipes.1.FBE.cs index 2fc733df..a8b6a118 100644 --- a/src/FactorioTools/OilField/Steps/AddPipes.1.FBE.cs +++ b/src/FactorioTools/OilField/Steps/AddPipes.1.FBE.cs @@ -219,24 +219,43 @@ private static Result DelaunayTriangulation(Context context, Loca break; } - var locationToGroup = groups.ToDictionary(context, x => x.Location, x => x); - locationToGroup.Add(group.Location, group); + // A group's Location is the rounded centroid of its pumpjack centers, so + // two different groups can land on the same tile. Keying a dictionary by it + // therefore has to allow more than one group per key. It used to use Add, + // which throws ArgumentException on the second one and takes the whole plan + // down. That is rare - it happens on none of the 1147 big-list blueprints + // as they are grouped today - but it is reachable, and a crash is the worst + // possible way to find out. + var locationToGroups = context.GetLocationDictionary>(groups.Count + 1); + for (var i = 0; i < groups.Count; i++) + { + IndexGroup(locationToGroups, groups[i]); + } - var groupLines = PointsToLines(locationToGroup.Keys); + IndexGroup(locationToGroups, group); + + var groupLines = PointsToLines(locationToGroups.Keys); var par = new List(groupLines.Count); for (var i = 0; i < groupLines.Count; i++) { var line = groupLines[i]; if (line.A == group.Location) { - par.Add(locationToGroup[line.B]); + AddPartners(par, locationToGroups[line.B], group); } else if (line.B == group.Location) { - par.Add(locationToGroup[line.A]); + AddPartners(par, locationToGroups[line.A], group); } } + // The triangulation sees one point for every group sharing this group's own + // centroid, so those groups can never turn up as the far end of a line. They + // are also the closest partners there are, so add them directly. When no two + // groups share a centroid this adds nothing, which is why the change leaves + // every existing plan untouched. + AddPartners(par, locationToGroups[group.Location], group); + var result = GetPathBetweenGroups( context, par, @@ -671,6 +690,53 @@ private record TwoConnectedGroups(List> Lines, int MinDistance, G private record PathAndTurns(Endpoints Endpoints, List Path, int Turns, int OriginalIndex); + /// + /// Files a group under its centroid, creating the bucket on first use. Several + /// groups can share a centroid, so the value is a list rather than a single group. + /// + private static void IndexGroup(ILocationDictionary> locationToGroups, Group group) + { + if (!locationToGroups.TryGetValue(group.Location, out var list)) + { + list = new List(1); + locationToGroups.Add(group.Location, list); + } + + list.Add(group); + } + + /// + /// Appends every candidate that is not the group being connected and is not already + /// present. The duplicate check matters because a group can be reached both through a + /// triangulation line and through sharing a centroid. + /// + private static void AddPartners(List partners, List candidates, Group self) + { + for (var i = 0; i < candidates.Count; i++) + { + var candidate = candidates[i]; + if (candidate == self) + { + continue; + } + + var alreadyAdded = false; + for (var j = 0; j < partners.Count; j++) + { + if (partners[j] == candidate) + { + alreadyAdded = true; + break; + } + } + + if (!alreadyAdded) + { + partners.Add(candidate); + } + } + } + private class Group { private readonly ILocationSet _terminals; diff --git a/src/lua/FactorioTools/AddPipes_1_FBE.lua b/src/lua/FactorioTools/AddPipes_1_FBE.lua index b57ff900..363d276a 100644 --- a/src/lua/FactorioTools/AddPipes_1_FBE.lua +++ b/src/lua/FactorioTools/AddPipes_1_FBE.lua @@ -38,7 +38,8 @@ System.namespace("Knapcode.FactorioTools.OilField", function (namespace) -- Teoxoy came up with the idea to use Delaunay triangulation for this problem. Awesome! -- namespace.class("AddPipesFbe", function (namespace) - local Execute, DelaunayTriangulation, GetNextLine, LineContainsAnAddedPumpjack, GetPathBetweenGroups, ConnectTwoGroups, class + local Execute, DelaunayTriangulation, GetNextLine, LineContainsAnAddedPumpjack, GetPathBetweenGroups, ConnectTwoGroups, IndexGroup, AddPartners, + class namespace.class("FbeResult", function (namespace) local __members__, __ctor__ __ctor__ = function (this, Pipes, FinalStrategy) @@ -462,24 +463,38 @@ System.namespace("Knapcode.FactorioTools.OilField", function (namespace) break end - local locationToGroup = KnapcodeFactorioTools.CollectionExtensions.ToDictionary(groups, context, function (x) - return x.Location - end, function (x) - return x - end, class.Group, class.Group) - locationToGroup:Add(group.Location, group) + -- A group's Location is the rounded centroid of its pumpjack centers, so + -- two different groups can land on the same tile. Keying a dictionary by it + -- therefore has to allow more than one group per key. It used to use Add, + -- which throws ArgumentException on the second one and takes the whole plan + -- down. That is rare - it happens on none of the 1147 big-list blueprints + -- as they are grouped today - but it is reachable, and a crash is the worst + -- possible way to find out. + local locationToGroups = context:GetLocationDictionary1(#groups + 1, ListGroup) + for i = 0, #groups - 1 do + IndexGroup(locationToGroups, groups:get(i)) + end + + IndexGroup(locationToGroups, group) - local groupLines = KnapcodeOilField.Helpers.PointsToLines(locationToGroup:getKeys()) + local groupLines = KnapcodeOilField.Helpers.PointsToLines(locationToGroups:getKeys()) local par = ListGroup(#groupLines) for i = 0, #groupLines - 1 do local line = groupLines:get(i) if KnapcodeOilField.Location.op_Equality(line.A, group.Location) then - par:Add(locationToGroup:get(line.B)) + AddPartners(par, locationToGroups:get(line.B), group) elseif KnapcodeOilField.Location.op_Equality(line.B, group.Location) then - par:Add(locationToGroup:get(line.A)) + AddPartners(par, locationToGroups:get(line.A), group) end end + -- The triangulation sees one point for every group sharing this group's own + -- centroid, so those groups can never turn up as the far end of a line. They + -- are also the closest partners there are, so add them directly. When no two + -- groups share a centroid this adds nothing, which is why the change leaves + -- every existing plan untouched. + AddPartners(par, locationToGroups:get(group.Location), group) + local result = GetPathBetweenGroups(context, par, group, 2 + maxTries - tries, strategy) if result.Exception ~= nil then return KnapcodeFactorioTools.Result.NewException(result.Exception, class.FbeResultInfo) @@ -896,6 +911,52 @@ System.namespace("Knapcode.FactorioTools.OilField", function (namespace) return KnapcodeFactorioTools.Result.NewData(class.TwoConnectedGroups(lines, minCount, a), class.TwoConnectedGroups) end + -- + -- Files a group under its centroid, creating the bucket on first use. Several + -- groups can share a centroid, so the value is a list rather than a single group. + -- + IndexGroup = function (locationToGroups, group) + local default, list = locationToGroups:TryGetValue(group.Location) + if not default then + list = ListGroup(1) + locationToGroups:Add(group.Location, list) + end + + list:Add(group) + end + -- + -- Appends every candidate that is not the group being connected and is not already + -- present. The duplicate check matters because a group can be reached both through a + -- triangulation line and through sharing a centroid. + -- + AddPartners = function (partners, candidates, self) + for i = 0, #candidates - 1 do + local continue + repeat + local candidate = candidates:get(i) + if candidate == self then + continue = true + break + end + + local alreadyAdded = false + for j = 0, #partners - 1 do + if partners:get(j) == candidate then + alreadyAdded = true + break + end + end + + if not alreadyAdded then + partners:Add(candidate) + end + continue = true + until 1 + if not continue then + break + end + end + end class = { Execute = Execute }