refactor: Material designator with less boilerplate#5221
Merged
paulgessinger merged 8 commits intoacts-project:mainfrom Mar 12, 2026
Merged
refactor: Material designator with less boilerplate#5221paulgessinger merged 8 commits intoacts-project:mainfrom
paulgessinger merged 8 commits intoacts-project:mainfrom
Conversation
Replace the double-dispatch pattern in MaterialDesignator.hpp with a std::variant holding the concrete designator types. DesignatorBase, NullDesignator, and the merged() virtual dispatch infrastructure are removed entirely. Each designator type becomes a standalone value type with plain apply(), graphvizLabel(), and merged() methods. Merging, applying, and graphviz output are handled by free functions operating on the variant via std::visit, with std::monostate as the identity (null) state. MaterialDesignatorBlueprintNodeImpl now stores a Designator value instead of a unique_ptr<DesignatorBase>, removing the heap allocation for the null state and the runtime polymorphism overhead.
…/diamond support Merge CylinderProtoDesignator and CuboidProtoDesignator into a single ProtoDesignator<FaceEnum, ShellType> template, mirroring the existing ISurfaceMaterialDesignator pattern. Axis validation and duplicate-face detection are separated into validateAxes() and validateDuplicate(), with geometry-specific logic guarded by if constexpr. Extend ISurfaceMaterialDesignator::shellTypeName() with cases for TrapezoidPortalShell and DiamondPortalShell, add the corresponding type aliases, and include both in the Designator variant so that the existing merge/apply/graphvizLabel free functions cover them for free.
paulgessinger
changed the title
Contributor
dimitra97
reviewed
Mar 9, 2026
Avoid static local in portalShellShapeName by moving the regex to an anonymous namespace. Keeps single construction while satisfying static-analysis tools.
dimitra97
previously approved these changes
Mar 10, 2026
andiwand
reviewed
Mar 10, 2026
andiwand
reviewed
Mar 10, 2026
Contributor
andiwand
left a comment
andiwand
left a comment
There was a problem hiding this comment.
there are a couple more sonarcloud comments that can be considered
- Add default case to Face switch for exhaustive handling (SonarCloud S131) - Use explicit template lambdas in std::visit over Designator - Move portalShell regex to file scope, mark validateAxes const - Use if-init in portalShellShapeName
acts-policybot
bot
dismissed
dimitra97’s stale review
Invalidated by push of 69500fe
dimitra97
approved these changes
Mar 12, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
std::variant–DesignatorBasepolymorphism is replaced by astd::variant-based design.ProtoDesignator– Single template for cylinder, cuboid, trapezoid, and diamond bounds.boost::core::demangle– RemoveshellTypeName()and useboost::core::demanglefor error messages.