Bugfixes and refactor for LogWarning/Error/Note#359
Open
LindyHopperGT wants to merge 5 commits intoMothCocoon:5.xfrom
Open
Bugfixes and refactor for LogWarning/Error/Note#359LindyHopperGT wants to merge 5 commits intoMothCocoon:5.xfrom
LindyHopperGT wants to merge 5 commits intoMothCocoon:5.xfrom
Conversation
## Core Type System - FFlowPinConnectionPolicy: Flexible pin connectivity policy framework - FFlowPinTypeMatchPolicy: Per-type matching rules with EFlowPinTypeMatchRules flags - Standard policy presets: VeryRelaxed, Relaxed, Strict, VeryStrict ### Graph Schema & Pin Connectivity - UFlowGraphSchema::ArePinTypesCompatible(): Policy-driven compatibility checking ### Flow Asset & Runtime - FFlowPinConnectionPolicy integration in UFlowAsset (was in UFlowGraphSchema, editor only, and not easily configurable for projects) - UFlowAsset::GetFlowPinConnectionPolicy(): Runtime policy access for instances ### Predicates & Data Validation - Now supports all FFlowDataPinValue types - Generic equality template: TryCheckResolvedValuesEqual<TFlowPinType>() - Validation with operator/type compatibility checks ### Settings & Configuration - FInstancedStruct-based policy selection in UFlowSettings - Per-asset-subclass policy override capability
[Flow] Replaced the stubbed, nonfunctional preload mechanism in FlowGraph with a policy-driven, async-safe & per-project extendable system. Core Interface (IFlowPreloadableInterface) - Replaced ad-hoc PreloadContentAsync (TFunction callback) with PreloadContent() → EFlowPreloadResult (Completed / PreloadInProgress) and FlushContent(). - Async C++ nodes return PreloadInProgress and call NotifyPreloadComplete() from their completion delegate. - Async Blueprint nodes override K2_PreloadContent and call NotifyPreloadComplete() on self. - Sync nodes return Completed unchanged. Policy System (FFlowPreloadPolicy, FFlowPinConnectionPolicy) - Introduced FFlowPreloadPolicy instanced-struct policy controlling per-node preload timing (OnGraphInitialize, OnActivate, ManualOnly, Never) and flush timing (OnGraphDeinitialize, OnNodeFinish, ManualOnly, Never). - UFlowSettings exposes default policies as nullable const T* accessors. UFlowAsset holds a resolved policy instance with check() validation. Preload Helper (FFlowPreloadHelper / FFlowPreloadHelper_Standard) - New instanced-struct helper allocated per-node at InitializeInstance for any node (or addon) implementing IFlowPreloadableInterface. - Tracks PendingPreloadCount (not a simple bool) so node + multiple addon participants are counted atomically before any PreloadContent call fires — prevents premature AllPreloadsComplete. - Lifecycle hooks: OnNodeInitializeInstance, OnNodeActivate, OnNodeCleanup, OnNodeDeinitializeInstance, OnNodeExecuteInput. - Safety flush on DeinitializeInstance regardless of flush timing policy. Context Pins - Preloadable nodes gain Preload Content and Flush Content exec input pins and All Preloads Complete exec output pin automatically via GetContextInputs/GetContextOutputs. - AllPreloadsComplete fires only when all participants (node + all preloadable addons) have reported completion. AddOn Participation - TryInitializePreloadHelper allocates a helper when any addon implements IFlowPreloadableInterface, even if the node itself does not. - TriggerPreload / TriggerFlush iterate preloadable addons alongside the node. - Added UFlowNodeAddOn::NotifyPreloadComplete() (BlueprintCallable) — delegates to owning node, mirroring the Finish/TriggerOutput pattern. Implementing Nodes UFlowNode_PlayLevelSequence — stores FStreamableHandle, binds CreateWeakLambda → NotifyPreloadComplete(), returns PreloadInProgress; FlushContent cancels the handle. UFlowNode_ExecuteComponent — delegates to component; PreloadInProgress from a component is guarded with ensureAlwaysMsgf (no callback path exists) and treated as Completed. UFlowNode_SubGraph — synchronous CreateSubFlow, returns Completed. CR - ECalder, BJarvinen
| // Often ExecuteInput is replaced rather than extended in subclasses. | ||
| // So any subclasses that implement the preload interface will want to call this function | ||
| // in their ExecuteInput() override. | ||
| if (DispatchExecuteInputToPreloadHelper(PinName)) |
There was a problem hiding this comment.
I have a small suggestion.
- Declare DispatchExecuteInputToPreloadHelper as a FlowNodeBase virtual method
- Make it return false in the default implementation
- Keep the same implementation for FlowNode, like the one you have here
- Remove the DispatchExecuteInputToPreloadHelper call from the UFlowNode::ExecuteInput
- Place it under the UFlowNodeBase::ExecuteInputForSelfAndAddOns instead, so we may get something like this
void UFlowNodeBase::ExecuteInputForSelfAndAddOns(const FName& PinName)
{
// AddOns can introduce input pins to Nodes without the Node being aware of the addition.
// To ensure that Nodes and AddOns only get the input pins signaled that they expect,
// we are filtering the PinName vs. the expected InputPins before carrying on with the ExecuteInput
if (IsSupportedInputPinName(PinName) && !DispatchExecuteInputToPreloadHelper(PinName))
{
ExecuteInput(PinName);
}
for (UFlowNodeAddOn* AddOn : AddOns)
{
AddOn->ExecuteInputForSelfAndAddOns(PinName);
}
}
There are two main reasons why I think it may work better:
- Preload/Flush are not just regular execution pins. We do not expect them to perform any additional logic except for managing the resources. In such a way, it will be easier to prevent human mistakes when we forget to protect the main execution block from being called by Preload/Flush pins
- As you mentioned here, sometimes ExecuteInput is replaced rather than extended in subclasses. And with this new approach, we don't need to duplicate the DispatchExecuteInputToPreloadHelper for such nodes
What do you think?
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
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.
Bug Fix - CancelAndWarnForUnflushedDeferredTriggers()
Bug Fix - TryFindActorOwner()
Refactor - LogError/LogWarning/LogNote