docs: address spec review feedback for abstract nest engine
Fix MainForm callsite descriptions, clarify default implementations return empty lists, make FillExact non-virtual, document PackArea signature refactor, add AutoNester scope note, specify error handling for plugin loading, document thread safety and instance lifetime. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -24,18 +24,22 @@ NestEngineRegistry (static, OpenNest.Engine)
|
||||
└── Factory method: Create(Plate) → NestEngineBase
|
||||
```
|
||||
|
||||
**Note on AutoNester:** The existing `AutoNester` static class (NFP + simulated annealing for mixed parts) is a natural future candidate for the registry but is currently unused by any caller. It is out of scope for this refactor — it can be wrapped as an engine later when it's ready for use.
|
||||
|
||||
## NestEngineBase
|
||||
|
||||
Abstract base class in `OpenNest.Engine`. Provides the contract, shared state, and utility methods.
|
||||
|
||||
**Instance lifetime:** Engine instances are short-lived and plate-specific — created per operation via the registry factory. Some engines (like `DefaultNestEngine`) maintain internal state across multiple `Fill` calls on the same instance (e.g., `knownGoodAngles` for angle pruning). Plugin authors should be aware that a single engine instance may receive multiple `Fill` calls within one nesting session.
|
||||
|
||||
### Properties
|
||||
|
||||
| Property | Type | Notes |
|
||||
|----------|------|-------|
|
||||
| `Plate` | `Plate` | The plate being nested |
|
||||
| `PlateNumber` | `int` | For progress reporting |
|
||||
| `NestDirection` | `NestDirection` | Fill direction preference |
|
||||
| `WinnerPhase` | `NestPhase` | Which phase produced the best result (private set) |
|
||||
| `NestDirection` | `NestDirection` | Fill direction preference, set by callers after creation |
|
||||
| `WinnerPhase` | `NestPhase` | Which phase produced the best result (protected set) |
|
||||
| `PhaseResults` | `List<PhaseResult>` | Per-phase results for diagnostics |
|
||||
| `AngleResults` | `List<AngleResult>` | Per-angle results for diagnostics |
|
||||
|
||||
@@ -48,7 +52,7 @@ Abstract base class in `OpenNest.Engine`. Provides the contract, shared state, a
|
||||
|
||||
### Virtual Methods (return parts, no side effects)
|
||||
|
||||
These are the core methods subclasses override:
|
||||
These are the core methods subclasses override. Base class default implementations return empty lists — subclasses override the ones they support.
|
||||
|
||||
```csharp
|
||||
virtual List<Part> Fill(NestItem item, Box workArea,
|
||||
@@ -57,14 +61,13 @@ virtual List<Part> Fill(NestItem item, Box workArea,
|
||||
virtual List<Part> Fill(List<Part> groupParts, Box workArea,
|
||||
IProgress<NestProgress> progress, CancellationToken token)
|
||||
|
||||
virtual List<Part> FillExact(NestItem item, Box workArea,
|
||||
IProgress<NestProgress> progress, CancellationToken token)
|
||||
|
||||
virtual List<Part> PackArea(Box box, List<NestItem> items,
|
||||
IProgress<NestProgress> progress, CancellationToken token)
|
||||
```
|
||||
|
||||
Default implementations in the base class delegate to `DefaultNestEngine`'s logic (or return empty lists — see DefaultNestEngine below).
|
||||
**`FillExact` is non-virtual.** It is orchestration logic (binary search wrapper around `Fill`) that works regardless of the underlying fill algorithm. It lives in the base class and calls the virtual `Fill` method. Any engine that implements `Fill` gets `FillExact` for free.
|
||||
|
||||
**`PackArea` signature change:** The current `PackArea(Box, List<NestItem>)` mutates the plate directly and returns `bool`. The new virtual method adds `IProgress<NestProgress>` and `CancellationToken` parameters and returns `List<Part>` (side-effect-free). This is a deliberate refactor — the old mutating behavior moves to the convenience overload `Pack(List<NestItem>)`.
|
||||
|
||||
### Convenience Overloads (non-virtual, add parts to plate)
|
||||
|
||||
@@ -95,7 +98,9 @@ Rename of the current `NestEngine`. Inherits `NestEngineBase` and overrides all
|
||||
|
||||
- `Name` → `"Default"`
|
||||
- `Description` → `"Multi-phase nesting (Linear, Pairs, RectBestFit, Remainder)"`
|
||||
- All current private methods (`FindBestFill`, `FillWithPairs`, `FillRectangleBestFit`, `FillPattern`, `TryRemainderImprovement`, `BuildCandidateAngles`, etc.) remain as private methods in this class
|
||||
- All current private methods (`FindBestFill`, `FillWithPairs`, `FillRectangleBestFit`, `FillPattern`, `TryRemainderImprovement`, `BuildCandidateAngles`, `QuickFillCount`, etc.) remain as private methods in this class
|
||||
- `ForceFullAngleSweep` property stays on `DefaultNestEngine` (not the base class) — only used by `BruteForceRunner` which references `DefaultNestEngine` directly
|
||||
- `knownGoodAngles` HashSet stays as a private field — accumulates across multiple `Fill` calls for angle pruning
|
||||
- No behavioral change — purely structural refactor
|
||||
|
||||
## StripNestEngine
|
||||
@@ -109,7 +114,7 @@ The planned `StripNester` (from the strip nester spec) becomes a `NestEngineBase
|
||||
|
||||
## NestEngineRegistry
|
||||
|
||||
Static class in `OpenNest.Engine` managing engine discovery and selection.
|
||||
Static class in `OpenNest.Engine` managing engine discovery and selection. Accessed only from the UI thread — not thread-safe. Engines are created per-operation and used on background threads, but the registry itself is only mutated/queried from the UI thread at startup and when the user changes the active engine.
|
||||
|
||||
### NestEngineInfo
|
||||
|
||||
@@ -146,7 +151,13 @@ Follows the existing `IPostProcessor` pattern from `Posts/`:
|
||||
- Reflect over types, find concrete subclasses of `NestEngineBase`
|
||||
- Require a constructor taking `Plate`
|
||||
- Register each with its `Name` and `Description` properties
|
||||
- Called at application startup alongside post-processor loading
|
||||
- Called at application startup alongside post-processor loading (WinForms app only — Console and MCP use built-in engines only)
|
||||
|
||||
**Error handling:**
|
||||
- DLLs that fail to load (bad assembly, missing dependencies) are logged and skipped
|
||||
- Types without a `Plate` constructor are skipped
|
||||
- Duplicate engine names: first registration wins, duplicates are logged and skipped
|
||||
- Exceptions from plugin constructors during `Create()` are caught and surfaced to the caller
|
||||
|
||||
## Callsite Migration
|
||||
|
||||
@@ -154,7 +165,7 @@ All `new NestEngine(plate)` calls become `NestEngineRegistry.Create(plate)`:
|
||||
|
||||
| Location | Count | Notes |
|
||||
|----------|-------|-------|
|
||||
| `MainForm.cs` | 3 | Auto-nest, fill plate, fill area |
|
||||
| `MainForm.cs` | 3 | Auto-nest fill, auto-nest pack, single-drawing fill plate |
|
||||
| `ActionFillArea.cs` | 2 | |
|
||||
| `PlateView.cs` | 1 | |
|
||||
| `NestingTools.cs` (MCP) | 6 | |
|
||||
|
||||
Reference in New Issue
Block a user