fix(blueprint): prevent editor crash creating ConstructObjectFromClass nodes (SpawnActorFromClass)#500
fix(blueprint): prevent editor crash creating ConstructObjectFromClass nodes (SpawnActorFromClass)#500Fl0p wants to merge 2 commits into
Conversation
…s nodes create_node hard-crashed the editor for any UK2Node_ConstructObjectFromClass subclass (K2Node_SpawnActorFromClass, CreateWidget, ...). Their PostPlacedNewNode() reads a checked pin accessor — e.g. UK2Node_SpawnActorFromClass::GetScaleMethodPin() => FindPinChecked() — before the generic create_node path allocated any pins, so the engine check() fired and killed the editor. The editor's own palette spawn survives because the cached template node already carries pins by the time PostPlacedNewNode runs. Route the whole ConstructObjectFromClass family through a dedicated TryCreateConstructObjectNode() that allocates default pins BEFORE PostPlacedNewNode(). It optionally seeds a construct/spawn class from the spawnClass/actorClass/class payload field and reconstructs the node to build the expose-on-spawn pins. The generic path (and its FunctionResult double-pin guard) is left unchanged. Fixes ChiR24#499 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
👋 Thanks for your first Pull Request! We love contributions. Please ensure you have signed off your commits and followed the contribution guidelines. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds an editor-only helper for creating ChangesConstruct-object node creation
Sequence Diagram(s)sequenceDiagram
participant CreateDynamicNode
participant TryCreateConstructObjectNode
participant TargetGraph
participant Blueprint
CreateDynamicNode->>TryCreateConstructObjectNode: Try construct-object node handling
TryCreateConstructObjectNode->>TargetGraph: AddNode(), CreateNewGuid(), AllocateDefaultPins(), PostPlacedNewNode()
TryCreateConstructObjectNode->>Blueprint: MarkBlueprintAsModified(), SaveLoadedAssetThrottled()
TryCreateConstructObjectNode-->>CreateDynamicNode: Success response with nodeId, nodeName, nodeClass
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/BlueprintGraph/McpAutomationBridge_BlueprintGraphHandlersSpecialNodes.cpp`:
- Around line 230-277: The special node creation path in
McpAutomationBridge_BlueprintGraphHandlersSpecialNodes should reject invalid
spawn class payloads instead of silently succeeding with an untyped node. In the
node setup flow around NewNode, resolve the requested class from
spawnClass/actorClass/class before mutating the graph, and if ResolveUClass()
fails, return an error via Context.SendError with CLASS_NOT_FOUND rather than
calling AddNode() and continuing. Keep the existing
AllocateDefaultPins/PostPlacedNewNode/ReconstructNode logic in the successful
path, but only after the class is validated and applied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 753dd497-66cf-49d9-954f-6216f2ff2dab
📒 Files selected for processing (3)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/BlueprintGraph/McpAutomationBridge_BlueprintGraphHandlersNodeCreation.cppplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/BlueprintGraph/McpAutomationBridge_BlueprintGraphHandlersPrivate.hplugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/BlueprintGraph/McpAutomationBridge_BlueprintGraphHandlersSpecialNodes.cpp
Per CodeRabbit PR review on ChiR24#500: TryCreateConstructObjectNode silently created an untyped node and returned success when spawnClass/actorClass/class was provided but ResolveUClass() could not resolve it, dropping the caller's requested target. Resolve the class up front, before mutating the graph, and return CLASS_NOT_FOUND when an explicitly requested class is unknown — matching the Cast branch convention in the same file. An absent class still yields a valid classless node. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
Hey! Nice catch on the root cause — allocating default pins before Two things before we merge though:
|
Summary
Fixes #499 —
manage_blueprint→create_nodehard-crashes the editor forK2Node_SpawnActorFromClass(and the rest of theUK2Node_ConstructObjectFromClassfamily:
ConstructObjectFromClass,CreateWidget, …).Root cause (corrected vs. the issue)
The issue blamed
AllocateDefaultPins()+FindPinCheckedand suggestedFGraphNodeCreator.That attribution is off by one call — and
FGraphNodeCreatorwould not have fixed it (itsFinalize()runs the same fatal order). The real culprit isPostPlacedNewNode():CreateDynamicNodecallsPostPlacedNewNode()whilePins.Num() == 0, so the checked pinaccessor asserts and the editor dies. The engine's own palette spawn survives because the cached
template node already carries pins by the time
PostPlacedNewNoderuns on the placed copy.Fix
Add
TryCreateConstructObjectNode()(in…SpecialNodes.cpp), dispatched fromCreateDynamicNoderight after the existing
TryCreateEnhancedInputNodecheck. It:NodeClass->IsChildOf(UK2Node_ConstructObjectFromClass::StaticClass())— covers thewhole family in one place;
PostPlacedNewNode()(the core fix), mirroring theproduction-proven
TryCreateEnhancedInputNodepath (Modify,ForceVisualizationCacheClear,NotifyGraphChanged, throttled save);spawnClass/actorClass/classpayload field(
ClassPin->DefaultObject+ReconstructNode()) so the expose-on-spawn pins are generated — aclassless node is still valid and no longer crashes;
NewObjectpath and itsFunctionResultdouble-pin guard untouched.Files
…/BlueprintGraph/McpAutomationBridge_BlueprintGraphHandlersSpecialNodes.cpp— new helper +#include "K2Node_ConstructObjectFromClass.h"…/BlueprintGraph/McpAutomationBridge_BlueprintGraphHandlersPrivate.h— declaration…/BlueprintGraph/McpAutomationBridge_BlueprintGraphHandlersNodeCreation.cpp— wire into dispatchNo
.Build.cschange (BlueprintGraph already a private dependency). No TS-bridge change required;forwarding the optional class field through the bridge schema is a possible follow-up.
Testing
create_node nodeType=K2Node_SpawnActorFromClass→ node created, editor stays alive (was 2/2 crash on UE 5.6).spawnClasspayload → expose-on-spawn pins appear;connect_pinsworks; blueprint compiles.K2Node_FunctionResultstill yields a single execute pin; simple nodes (get/set, math) unaffected.ConstructObjectFromClass,CreateWidget.Root cause verified against UE5 source and a working reference plugin (allocates pins first, skips
PostPlacedNewNodeentirely); the order-swap is class-scoped and low-risk. I haven't been able torun a full UE 5.6 editor build on my end — happy to iterate if your CI/build surfaces anything.
🤖 Generated with Claude Code