Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/NwcIO/Model/Clash/NwcClashDetector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ public static class NwcClashDetector
/// <summary>
/// Executes clash detection across an individual Navisworks document according to the specified test configuration.
/// </summary>
/// <exception cref="ArgumentException">A selection names a set that the document doesn't contain.</exception>
public static NwcClashTest Run(NwcDocument document, NwcClashTest test)
{
ArgumentNullException.ThrowIfNull(document);
ArgumentNullException.ThrowIfNull(test);
ThrowIfSetIsMissing([document], test);

var meshesA = ResolveSelection(document, test.SelectionA);
var meshesB = ResolveSelection(document, test.SelectionB);
Expand All @@ -27,10 +29,12 @@ public static NwcClashTest Run(NwcDocument document, NwcClashTest test)
/// <summary>
/// Executes clash detection across all contributing models of a federated Navisworks assembly.
/// </summary>
/// <exception cref="ArgumentException">A selection names a set that none of the models contains.</exception>
public static NwcClashTest Run(NwcAssembly assembly, NwcClashTest test)
{
ArgumentNullException.ThrowIfNull(assembly);
ArgumentNullException.ThrowIfNull(test);
ThrowIfSetIsMissing(assembly.Models, test);

var itemsA = new List<(NwcMesh Mesh, string ModelName, string? Layer)>();
var itemsB = new List<(NwcMesh Mesh, string ModelName, string? Layer)>();
Expand Down Expand Up @@ -350,9 +354,29 @@ private static IReadOnlyList<NwcMesh> ResolveSelection(NwcDocument document, Nwc
matchedMeshes.AddRange(document.Meshes.Where(m => m.Node is not null && nodeSet.Contains(m.Node)));
}
}
if (matchedMeshes.Count > 0) return matchedMeshes;
// Only the named sets, even when they match nothing: a model without them adds no meshes
return matchedMeshes;
}

return document.Meshes;
}

/// <summary>
/// Throws when a set named by a selection is in none of the models, so a misspelled name can't run against
/// nothing without notice. A set that exists in only some models of an assembly is fine.
/// </summary>
private static void ThrowIfSetIsMissing(IReadOnlyList<NwcDocument> documents, NwcClashTest test)
{
foreach (var selection in new[] { test.SelectionA, test.SelectionB })
{
foreach (string name in selection.SetNames)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Validate set names only when that selection branch is active

ResolveSelection gives ExplicitNodeIds precedence over FilterCondition and SetNames, but this new validation checks every set name unconditionally.

Reproduction: set SelectionA.ExplicitNodeIds = [1] (via Add/AddRange) and leave SelectionA.SetNames = ["unused missing set"]. With two overlapping meshes, the current main/base completes and returns one clash because the set-name branch is unused; this PR throws ArgumentException before resolving the explicit IDs. The same problem applies when FilterCondition overrides set names.

Please validate SetNames only when neither explicit IDs nor a filter condition determines the selection, and add a regression test for the existing precedence.

{
if (!documents.Any(d => d.FindSet(name) is not null))
{
throw new ArgumentException(
$"Clash test '{test.Name}' selects the set '{name}', which no model contains.", nameof(test));
}
}
}
}
}
72 changes: 72 additions & 0 deletions tests/NwcIO.Tests/ClashDetectiveTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,78 @@ public void AssemblyClashDetection_ReportsOnlyUniqueCrossModelPairs(bool interse
Assert.All(test.Results, clash => Assert.NotEqual(clash.Item1.ModelName, clash.Item2.ModelName));
}

private static NwcDocument CreateClashingDocument(string name, params Model.Sets.NwcSetItem[] sets)
{
var builder = new NwcDocumentBuilder().SetFileInfo(name)
.AddMesh(CreateBoxMesh(1, new(0, 0, 0), new(2, 2, 2)))
.AddMesh(CreateBoxMesh(2, new(1, 0, 0), new(3, 2, 2)));
foreach (var set in sets)
{
builder.AddSelectionSet(set);
}

return builder.Build();
}

[Fact]
public void SetSelection_UnknownSetName_Throws()
{
var doc = CreateClashingDocument("Model.nwc");
var test = new NwcClashTest("Typo", NwcClashTestType.Hard, tolerance: 0.01)
{
SelectionA = NwcClashSelection.FromSet("NoSuchSet"),
SelectionB = NwcClashSelection.All(),
};

var ex = Assert.Throws<ArgumentException>(() => NwcClashDetector.Run(doc, test));
Assert.Contains("NoSuchSet", ex.Message, StringComparison.Ordinal);
}

[Fact]
public void SetSelection_SetWithoutMeshes_SelectsNothing()
{
var doc = CreateClashingDocument("Model.nwc", new Model.Sets.NwcSelectionSet("Empty", [999]));
var test = new NwcClashTest("Empty set", NwcClashTestType.Hard, tolerance: 0.01)
{
SelectionA = NwcClashSelection.FromSet("Empty"),
SelectionB = NwcClashSelection.All(),
};

NwcClashDetector.Run(doc, test);

Assert.Equal(0, test.TotalClashCount);
}

[Fact]
public void SetSelection_SetInOneModelOnly_SelectsNothingFromTheOthers()
{
var architecture = CreateClashingDocument("Architecture.nwc", new Model.Sets.NwcSelectionSet("Empty", [999]));
var mep = CreateClashingDocument("MEP.nwc");
var test = new NwcClashTest("Federated", NwcClashTestType.Hard, tolerance: 0.01)
{
SelectionA = NwcClashSelection.FromSet("Empty"),
SelectionB = NwcClashSelection.All(),
};

NwcClashDetector.Run(NwcAssembly.Create("Federated", architecture, mep), test);

Assert.Equal(0, test.TotalClashCount);
}

[Fact]
public void SetSelection_SetMissingFromEveryModel_Throws()
{
var assembly = NwcAssembly.Create("Federated", CreateClashingDocument("A.nwc"), CreateClashingDocument("B.nwc"));
var test = new NwcClashTest("Typo", NwcClashTestType.Hard, tolerance: 0.01)
{
SelectionA = NwcClashSelection.All(),
SelectionB = NwcClashSelection.FromSet("NoSuchSet"),
};

var ex = Assert.Throws<ArgumentException>(() => NwcClashDetector.Run(assembly, test));
Assert.Contains("NoSuchSet", ex.Message, StringComparison.Ordinal);
}

[Fact]
public void IgnoreSameLayerRuleFiltersOutSameLayerClashes()
{
Expand Down
Loading