fix(umg): bind_on_hovered and bind_on_value_changed author real bound-event nodes#482
fix(umg): bind_on_hovered and bind_on_value_changed author real bound-event nodes#482SoloGorilla wants to merge 1 commit into
Conversation
…-event nodes
bind_on_hovered and bind_on_value_changed returned an instruction string ("Bind '...' to ...'s
... event.") instead of authoring a node, unlike bind_on_clicked which creates a real
UK2Node_ComponentBoundEvent.
Extracted the working bind_on_clicked path into a shared BindComponentDelegateEvent helper and
routed both actions through it:
- bind_on_hovered -> Button OnHovered.
- bind_on_value_changed -> per widget type: Slider/SpinBox OnValueChanged, CheckBox
OnCheckStateChanged, ComboBoxString OnSelectionChanged; unsupported widgets return
UNSUPPORTED_WIDGET instead of a misleading success.
Both now return a real node (nodeId + compileSucceeded), are idempotent (reuse an existing bound
event), and only dirty/recompile when something changed. create_property_binding is left unchanged.
Verified live in UE 5.8: bind_on_hovered and bind_on_value_changed (Slider) both produce real
K2Node_ComponentBoundEvent nodes with compileSucceeded:true.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughA new internal helper ChangesWidget Event Binding Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp (1)
25-116: ⚡ Quick winRoute
bind_on_clickedthrough the helper as well.The new helper duplicates the full
OnClickedbinding path, butbind_on_clickedstill keeps its own copy. That means fixes like the structural-dirtying issue above must be patched twice. The helper signature already supportsButtonWidget,UButton::StaticClass(), andOnClicked, so the clicked handler can delegate after validation.Also applies to: 159-237
🤖 Prompt for 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. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp` around lines 25 - 116, The bind_on_clicked handler is duplicating the full binding logic instead of using the new BindComponentDelegateEvent helper function that was created. Locate the bind_on_clicked handler implementation and refactor it to call BindComponentDelegateEvent with the appropriate parameters (TargetWidget as ButtonWidget, DelegateOwnerClass as UButton::StaticClass(), DelegateName as OnClicked, and other metadata) instead of implementing the same logic directly. This eliminates code duplication and ensures that any future fixes to the binding process only need to be applied once in the helper function.
🤖 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/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp`:
- Around line 85-100: When creating a new UK2Node_ComponentBoundEvent node in
the EventGraph, the code marks the Blueprint as modified via
MarkBlueprintAsModified but fails to mark it as structurally modified. Graph
node creation is a structural change to the Blueprint, so you need to add a call
to MarkBlueprintAsStructurallyModified in addition to MarkBlueprintAsModified.
Specifically, when bCreatedNew is true (indicating a new BoundNode was created),
call FBlueprintEditorUtils::MarkBlueprintAsStructurallyModified on the WidgetBP
before or after the MarkBlueprintAsModified call to properly notify the
Blueprint system of the structural change.
---
Nitpick comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp`:
- Around line 25-116: The bind_on_clicked handler is duplicating the full
binding logic instead of using the new BindComponentDelegateEvent helper
function that was created. Locate the bind_on_clicked handler implementation and
refactor it to call BindComponentDelegateEvent with the appropriate parameters
(TargetWidget as ButtonWidget, DelegateOwnerClass as UButton::StaticClass(),
DelegateName as OnClicked, and other metadata) instead of implementing the same
logic directly. This eliminates code duplication and ensures that any future
fixes to the binding process only need to be applied once in the helper
function.
🪄 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: 7deb5908-9bba-4d14-95af-530dde7d3ab7
📒 Files selected for processing (1)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp
| if (!BoundNode) | ||
| { | ||
| EventGraph->Modify(); | ||
| FGraphNodeCreator<UK2Node_ComponentBoundEvent> Creator(*EventGraph); | ||
| BoundNode = Creator.CreateNode(false); | ||
| BoundNode->InitializeComponentBoundEventParams(CompProp, DelegateProp); | ||
| Creator.Finalize(); | ||
| bCreatedNew = true; | ||
| bBlueprintChanged = true; | ||
| } | ||
|
|
||
| bool bCompiled = true; | ||
| if (bBlueprintChanged) | ||
| { | ||
| FBlueprintEditorUtils::MarkBlueprintAsModified(WidgetBP); | ||
| bCompiled = McpSafeCompileBlueprint(WidgetBP); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code file at the specified lines
cat -n plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp | sed -n '80,110p'Repository: ChiR24/Unreal_mcp
Length of output: 1628
🏁 Script executed:
# Get full context of the BindComponentDelegateEvent function
cat -n plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp | sed -n '25,120p'Repository: ChiR24/Unreal_mcp
Length of output: 4763
🏁 Script executed:
# Check for any other uses of MarkBlueprintAsStructurallyModified in this file or related files
rg "MarkBlueprintAsStructurallyModified" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Repository: ChiR24/Unreal_mcp
Length of output: 18120
🏁 Script executed:
# Search for UK2Node_ComponentBoundEvent usage in the codebase
rg "UK2Node_ComponentBoundEvent" plugins/McpAutomationBridge/Repository: ChiR24/Unreal_mcp
Length of output: 2127
🏁 Script executed:
# Check for other instances of MarkBlueprintAsModified vs MarkBlueprintAsStructurallyModified usage patterns in the codebase
rg "MarkBlueprintAsModified|MarkBlueprintAsStructurallyModified" plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/ -A 2 -B 2 | head -100Repository: ChiR24/Unreal_mcp
Length of output: 15813
Add MarkBlueprintAsStructurallyModified when creating bound-event nodes.
Creating UK2Node_ComponentBoundEvent is a structural Blueprint change. When TargetWidget->bIsVariable is already true, the code path skips the structural modification call at line 48 and proceeds to create the node at line 89. However, line 99 only calls MarkBlueprintAsModified, which is insufficient for graph node creation. The Blueprint's structural state must be marked dirty when the new event node is added.
Fix
Creator.Finalize();
bCreatedNew = true;
bBlueprintChanged = true;
+ FBlueprintEditorUtils::MarkBlueprintAsStructurallyModified(WidgetBP);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!BoundNode) | |
| { | |
| EventGraph->Modify(); | |
| FGraphNodeCreator<UK2Node_ComponentBoundEvent> Creator(*EventGraph); | |
| BoundNode = Creator.CreateNode(false); | |
| BoundNode->InitializeComponentBoundEventParams(CompProp, DelegateProp); | |
| Creator.Finalize(); | |
| bCreatedNew = true; | |
| bBlueprintChanged = true; | |
| } | |
| bool bCompiled = true; | |
| if (bBlueprintChanged) | |
| { | |
| FBlueprintEditorUtils::MarkBlueprintAsModified(WidgetBP); | |
| bCompiled = McpSafeCompileBlueprint(WidgetBP); | |
| if (!BoundNode) | |
| { | |
| EventGraph->Modify(); | |
| FGraphNodeCreator<UK2Node_ComponentBoundEvent> Creator(*EventGraph); | |
| BoundNode = Creator.CreateNode(false); | |
| BoundNode->InitializeComponentBoundEventParams(CompProp, DelegateProp); | |
| Creator.Finalize(); | |
| bCreatedNew = true; | |
| bBlueprintChanged = true; | |
| FBlueprintEditorUtils::MarkBlueprintAsStructurallyModified(WidgetBP); | |
| } | |
| bool bCompiled = true; | |
| if (bBlueprintChanged) | |
| { | |
| FBlueprintEditorUtils::MarkBlueprintAsModified(WidgetBP); | |
| bCompiled = McpSafeCompileBlueprint(WidgetBP); |
🤖 Prompt for 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.
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp`
around lines 85 - 100, When creating a new UK2Node_ComponentBoundEvent node in
the EventGraph, the code marks the Blueprint as modified via
MarkBlueprintAsModified but fails to mark it as structurally modified. Graph
node creation is a structural change to the Blueprint, so you need to add a call
to MarkBlueprintAsStructurallyModified in addition to MarkBlueprintAsModified.
Specifically, when bCreatedNew is true (indicating a new BoundNode was created),
call FBlueprintEditorUtils::MarkBlueprintAsStructurallyModified on the WidgetBP
before or after the MarkBlueprintAsModified call to properly notify the
Blueprint system of the structural change.
|
On adding |
1 participant
Problem
bind_on_hoveredandbind_on_value_changedreturned a plain instruction string (e.g."Bind 'OnButtonHovered' to BtnStart's OnHovered event.") instead of authoring anything — no node, nonodeId. Onlybind_on_clickedcreated a realUK2Node_ComponentBoundEvent.Fix (
EventBindings.cpp)Extracted the working
bind_on_clickedimplementation into a sharedBindComponentDelegateEventhelper and routed the other two through it:bind_on_hovered→ ButtonOnHovered.bind_on_value_changed→ per widget type: Slider/SpinBoxOnValueChanged, CheckBoxOnCheckStateChanged, ComboBoxStringOnSelectionChanged; unsupported widgets returnUNSUPPORTED_WIDGETrather than a misleading success.Both now author a real
UK2Node_ComponentBoundEvent(returnsnodeId+compileSucceeded), are idempotent (reuse an existing bound event viaFindBoundEventForComponent), and only dirty/recompile when something actually changed — matchingbind_on_clickedexactly.create_property_bindingis intentionally left unchanged (separate follow-up).Validation (UE 5.8, live)
bind_on_hovered(Button) andbind_on_value_changed(Slider) both produce realK2Node_ComponentBoundEventnodes (OnButtonHoverEvent/OnFloatValueChangedEvent) withcompileSucceeded:true.🤖 Generated with Claude Code