MothCocoon / FlowGraph

Design-agnostic node system for scripting game’s flow in Unreal Engine
https://discord.gg/Xmtr6GhbmW
MIT License
1.17k stars 228 forks source link

Node title text changes #193

Closed MaksymKapelianovych closed 2 months ago

MaksymKapelianovych commented 5 months ago
  1. Editing UFlowGraphEditorSettings::bShowNodeClass now immediately updates node titles in graph.
  2. Editing UFlowSettings::bUseAdaptiveNodeTitles now immediately updates node titles in graph.
  3. UFlowGraphNode::GetTooltipText() now calls UFlowNode::GetNodeToolTip()
  4. Refresh graph only once, when node asset is renamed
  5. User can now specify node prefixes that will be automatically removed, instead of manually writing custom meta = (DisplayName = ...)
MaksymKapelianovych commented 5 months ago

More about the latter. Now UFlowGraphSettings have NodePrefixesToRemove array, where user can add their custom node prefixes. By default it contains two elements: "FN" and "FlowNode". Any duplicating elements will be instantly removed with error notification. To optimize process of prefix removal, nodes' names without prefixes are generated and stored as custom "GeneratedDisplayName" metadata every time array is changed. UFlowNode::GetNodeTitle() method is changed to return "GeneratedDisplayName" in case node class does not have BlueprintDisplayName and "DisplayName" metadata, and UFlowNode::bDisplayNodeTitleWithoutPrefix == true. Similar changes have been made in UFlowNode::GetNodeToolTip(), so now it also returns "GeneratedDisplayName" if possible.

MaksymKapelianovych commented 5 months ago

Also, I would like to change one thing related to node titles, but decided not to include into this PR: move UFlowSettings::bUseAdaptiveNodeTitles to UFlowGraphEditorSettingsbecause it is just an Editor setting and has no influence on runtime.

But this will require to change the way of how UFlowNode returns its Title and Description. Let UFlowGraphNode decide which type of title it needs!) It can be done in two ways:

  1. Preferred way (by me). Make enum class EFlowNodeTitleType { Default, Adaptive }; and pass it as a parameter to UFlowNode::GetNodeTitle() and UFlowNode::GetNodeDescription(). In child classes this implementation will require changing UFlowSettings::Get()->bUseAdaptiveNodeTitles to EFlowNodeTitleType == Adaptive. This parameter can also be passed to K2_GetNodeDescription(), so BP Nodes also will be able to customize their title. But this change to function signature will require all plugin users to fix their nodes, otherwise code will not compile(
  2. Make two separate pair of functions UFlowNode::GetNodeTitle()/UFlowNode::GetNodeAdaptiveTitle() and UFlowNode::GetNodeDescription()/UFlowNode::GetNodeAdaptiveDescription(). UFlowGraphNodeand other classes will call required function. This change also will require all plugin users to make changes to their nodes, but at least code will compile.

Please, let me know which method you prefer and if none, why?

MothDoctor commented 2 months ago

Hi, please grab the latest changes as this change has merge conflicts with just accepted Flow AddOns.

MaksymKapelianovych commented 2 months ago

Updated!

MothDoctor commented 2 months ago

Awesome, change accepted :)

Also, I would like to change one thing related to node titles, but decided not to include into this PR: move UFlowSettings::bUseAdaptiveNodeTitles to UFlowGraphEditorSettingsbecause it is just an Editor setting and has no influence on runtime. Please, let me know which method you prefer and if none, why?

Sure, go for it! :) I would also prefer method 1. I browsed the code and I've seen many repeated settings check in many core nodes. It is indeed a minor boilerplate.

MaksymKapelianovych commented 2 months ago

Update.

I have started working on PR for moving UFlowSettings::bUseAdaptiveNodeTitles to UFlowGraphEditorSettings and what at first looked like an ok idea now looks terrible. It requires changing all overrides of GetNodeTitle and GetNodeDescription (42 files affected in total) just to slightly change condition... It could be done though, but with AddOn PR there was added UpdateNodeConfigText function, it can update config text, depending on whether title is adaptive or not and it is not called from UFlowGraphNode. So implementation of my planned change for this function would require either including FlowGraphEditorSettings.h and adding FlowEditor dependency to Flow module (obviously not good) or some way to cache current title type by FlowNode itself, which seems like an overengineering. Eventually, as this PR would bring almost no value, I decided not to make it.