Better error reporting in WingFetcher

Plugins/UEWingman/Source/UEWingman/Handlers/UserManual.h

@@ -3,6 +3,7 @@
 #include "CoreMinimal.h"
 #include "WingHandler.h"
 #include "WingServer.h"
+#include "WingFetcher.h"
 #include "UserManual.generated.h"
 
 UCLASS()
@@ -18,28 +19,8 @@ public:
 
 	virtual void Handle() override
 	{
+		WingFetcher::PrintPathExplanation();
 		UWingServer::Print(TEXT(
-			"\n PATHS:"
-			"\n"
-			"\n Most commands require you to specify a path.  A path starts"
-			"\n with an asset name, followed by steps separated by ,"
-			"\n that navigate into the asset.  Example:"
-			"\n"
-			"\n  /Game/Widgets/WB_Hotkeys,graph:EventGraph,node:Self03,pin:Result"
-			"\n"
-			"\n The navigation steps supported are:"
-			"\n"
-			"\n  graph — move from a blueprint or material to a graph."
-			"\n  node — move from a graph to a graph node"
-			"\n  pin — move from a graph node to a pin"
-			"\n  component — move from a blueprint to a component"
-			"\n  levelblueprint — move from a world to a blueprint"
-			"\n"
-			"\n Steps do not always require a parameter.  For example, materials"
-			"\n only have one graph, so you can just say:"
-			"\n"
-			"\n	 /Game/Materials/MyMaterial,graph"
-			"\n"
 			"\n TYPES:"
 			"\n"
 			"\n To change variable types, or function prototypes, you will"
@@ -70,6 +51,25 @@ public:
 			"\n before you can set return values.  Custom event nodes also have"
 			"\n editable arguments."
 			"\n"
+			"\n IDENTIFIER SANITIZATION:\n"
+			"\n"
+			"\n Identifiers in Unreal can contain spaces and punctuation marks.\n"
+			"\n Those punctuation marks could confuse our parsers.  For example,"
+			"\n How would we parse Array<X> if the typename X contained a less-than?"
+			"\n So, we automatically translate these characters on output:"
+			"\n"
+			"\n   space ->  ·"
+			"\n   <     ->  ◁"
+			"\n   >     ->  ▷"
+			"\n   ,     ->  ▾"
+			"\n "
+			"\n We do the reverse translation on input.  Therefore, you will always"
+			"\n see sanitized versions of identifiers, and you must always use"
+			"\n sanitized versions of identifiers:"
+			"\n"
+			"\n Correct: /Game/Testing/BP_Test,graph:Get·Cursor·Location"
+			"\n Wrong:   /Game/Testing/BP_Test,graph:Get Cursor Location"
+			"\n"
 			"\n ABOUT WHITESPACE:"
 			"\n"
 			"\n Do not put excess whitespace into paths, typenames, or"

Plugins/UEWingman/Source/UEWingman/Private/WingFetcher.cpp

@@ -19,6 +19,38 @@
 #include "Components/Widget.h"
 #include "Subsystems/AssetEditorSubsystem.h"
 
+void WingFetcher::PrintPathExplanation()
+{
+	UWingServer::Print(TEXT(
+		"\n PATHS:"
+		"\n"
+		"\n Most commands require you to specify a path.  A path starts"
+		"\n with an asset name, followed by steps separated by ,"
+		"\n that navigate into the asset.  Some Examples:"
+		"\n"
+		"\n  /Game/Widgets/WB_Hotkeys,graph:EventGraph,node:Self03,pin:Result"
+		"\n  /Game/Testing/BP_Test,graph:Clear·Action·Grid,node:K2Node_CallFunction_0"
+		"\n"
+		"\n The navigation steps supported are:"
+		"\n"
+		"\n  graph — move from a blueprint or material to a graph."
+		"\n  node — move from a graph to a graph node"
+		"\n  pin — move from a graph node to a pin"
+		"\n  component — move from a blueprint to a component"
+		"\n  levelblueprint — move from a world to a blueprint"
+		"\n  widget — move from a widget blueprint to a widget"
+		"\n"
+		"\n Notice that paths use sanitized identifiers.  See the UserManual"
+		"\n for more information on name sanitization." 
+		"\n"
+		"\n Steps do not always require a parameter.  For example, materials"
+		"\n only have one graph, so you can just say:"
+		"\n"
+		"\n	 /Game/Materials/MyMaterial,graph"
+		"\n"
+	));
+}
+
 WingFetcher::WalkFunc WingFetcher::GetWalker(const FString& Step)
 {
 	if (Step.Equals(TEXT("graph"),          ESearchCase::IgnoreCase)) return &WingFetcher::Graph;
@@ -56,24 +88,24 @@ WingFetcher& WingFetcher::SetError()
 
 void WingFetcher::PathFailed(const TCHAR* Expected)
 {
-	SetError();
 	if (ResultPin)
 		UWingServer::Printf(TEXT("ERROR: Path specifies a pin, but expected %s\n"), Expected);
 	else if (Obj)
 		UWingServer::Printf(TEXT("ERROR: Path specifies a %s, but expected %s\n"), *Obj->GetClass()->GetName(), Expected);
 	else
 		UWingServer::Printf(TEXT("ERROR: Path led to a null pointer\n"));
+	SetError();
 }
 
 WingFetcher& WingFetcher::TypeMismatch(const TCHAR* Walker, const TCHAR* Expected)
 {
-	SetError();
 	if (ResultPin)
 		UWingServer::Printf(TEXT("ERROR: Input to '%s' is a pin, but expected %s\n"), Walker, Expected);
 	else if (Obj)
 		UWingServer::Printf(TEXT("ERROR: Input to '%s' is %s, but expected %s\n"), Walker, *Obj->GetClass()->GetName(), Expected);
 	else
 		UWingServer::Printf(TEXT("ERROR: Path led to a null pointer\n"));
+	SetError();
 	return *this;
 }
 
@@ -119,6 +151,13 @@ WingFetcher& WingFetcher::Asset(const FString& PackagePath)
 {
 	if (bError) return *this;
 
+	if (!PackagePath.StartsWith(TEXT("/")))
+	{
+		UWingServer::Printf(TEXT("ERROR: Asset path must start with '/', got '%s'\n"), *PackagePath);
+		PrintPathExplanation();
+		return SetError();
+	}
+
 	// Check if the package exists before calling LoadObject, because
 	// LoadObject logs its own errors when the package doesn't exist.
 	FString PackageName = FPackageName::ObjectPathToPackageName(PackagePath);
@@ -182,7 +221,8 @@ WingFetcher& WingFetcher::Graph(const FString& Value)
 	{
 		if (!Value.IsEmpty())
 		{
-			UWingServer::Printf(TEXT("ERROR: Materials do not have named graphs (got '%s')\n"), *Value);
+			UWingServer::Printf(TEXT("ERROR: Materials have only one graph, with a blank name.\n\n"));
+			PrintPathExplanation();
 			return SetError();
 		}
 		WingUtils::EnsureMaterialGraph(Mat);
@@ -197,12 +237,26 @@ WingFetcher& WingFetcher::Graph(const FString& Value)
 
 	UBlueprint* BP = ::Cast<UBlueprint>(Obj);
 	if (!BP)
-		return TypeMismatch(TEXT("graph"), TEXT("Blueprint or Material"));
+	{
+		TypeMismatch(TEXT("graph"), TEXT("Blueprint or Material"));
+		PrintPathExplanation();
+		return SetError();
+	}
 
-	UEdGraph* Graph = WingUtils::FindExactlyOneNamed(Value, WingUtils::AllGraphs(BP), TEXT("Graph"));
+	TArray<UEdGraph*> Graphs = WingUtils::AllGraphs(BP);
-	if (!Graph) return SetError();
+	UEdGraph* Found = WingUtils::FindExactlyOneNamed(Value, Graphs, TEXT("graph"));
+	if (!Found)
+	{
+		UWingServer::Printf(TEXT("Graphs that exist in blueprint:"));
+		for (const UEdGraph *G : Graphs)
+		{
+			UWingServer::Printf(TEXT(" %s"), *WingUtils::FormatName(G));
+		}
+		UWingServer::Printf(TEXT("\n"));
+		return SetError();
+	}
 
-	SetObj(Graph);
+	SetObj(Found);
 	return *this;
 }
 
@@ -210,58 +264,30 @@ WingFetcher& WingFetcher::Node(const FString& Value)
 {
 	if (bError) return *this;
 
-	// If current object is a graph, search that graph
+	// If current object is not a graph, refuse.
-	if (UEdGraph* G = ::Cast<UEdGraph>(Obj))
+	UEdGraph *Graph = ::Cast<UEdGraph>(Obj);
+	if (Graph == nullptr)
 	{
-		UEdGraphNode* Found = nullptr;
+		TypeMismatch(TEXT("node"), TEXT("Graph"));
-		for (UEdGraphNode* N : G->Nodes)
+		PrintPathExplanation();
-		{
+		return SetError();
-			if (!N || !WingUtils::Identifies(Value, N))
-				continue;
-			if (Found)
-			{
-				UWingServer::Printf(TEXT("ERROR: Ambiguous node '%s' in graph %s\n"), *Value, *G->GetName());
-				return SetError();
-			}
-			Found = N;
-		}
-		if (!Found)
-		{
-			UWingServer::Printf(TEXT("ERROR: Node '%s' not found in graph %s\n"), *Value, *G->GetName());
-			return SetError();
-		}
-		SetObj(Found);
-		return *this;
 	}
 
-	// If current object is a blueprint, search all graphs
+	// Get the nodes from the graph.
-	if (UBlueprint* BP = ::Cast<UBlueprint>(Obj))
+	TArray<UEdGraphNode *> AllNodes = WingUtils::AllNodes(Graph);
+	UEdGraphNode *Node = WingUtils::FindExactlyOneNamed(Value, AllNodes, TEXT("node"));
+	if (Node == nullptr)
 	{
-		UEdGraphNode* Found = nullptr;
+		UWingServer::Printf(TEXT("Nodes that exist in graph:"));
-		for (UEdGraph* G : WingUtils::AllGraphs(BP))
+		for (const UEdGraphNode *N : AllNodes)
 		{
-			for (UEdGraphNode* N : G->Nodes)
+			UWingServer::Printf(TEXT(" %s"), *WingUtils::FormatName(N));
-			{
-				if (!N || !WingUtils::Identifies(Value, N))
-					continue;
-				if (Found)
-				{
-					UWingServer::Printf(TEXT("ERROR: Ambiguous node '%s' in %s\n"), *Value, *BP->GetName());
-					return SetError();
-				}
-				Found = N;
-			}
 		}
-		if (!Found)
+		UWingServer::Printf(TEXT("\n"));
-		{
+		return SetError();
-			UWingServer::Printf(TEXT("ERROR: Node '%s' not found in %s\n"), *Value, *BP->GetName());
-			return SetError();
-		}
-		SetObj(Found);
-		return *this;
 	}
-
+	SetObj(Node);
-	return TypeMismatch(TEXT("node"), TEXT("graph or Blueprint"));
+	return *this;
 }
 
 WingFetcher& WingFetcher::Pin(const FString& Value)
@@ -270,27 +296,22 @@ WingFetcher& WingFetcher::Pin(const FString& Value)
 
 	UEdGraphNode* N = ::Cast<UEdGraphNode>(Obj);
 	if (!N)
-		return TypeMismatch(TEXT("pin"), TEXT("node"));
-	UEdGraphPin* Found = nullptr;
-	for (UEdGraphPin *P : N->Pins)
 	{
-		if (!WingUtils::Identifies(Value, P))
+		TypeMismatch(TEXT("pin"), TEXT("node"));
-			continue;
+		PrintPathExplanation();
-		if (Found)
+		return SetError();
-		{
+	}
-			UWingServer::Printf(TEXT("ERROR: Ambiguous pin '%s' on node %s\n"),
+	UEdGraphPin *Found = WingUtils::FindExactlyOneNamed(Value, N->Pins, TEXT("pin"));
-					*Value, *WingUtils::FormatName(N));
+	if (!Found)
-			return SetError();
+	{
-		}
+		UWingServer::Printf(TEXT("Pins that exist in the node:"));
-		Found = P;
+		for (const UEdGraphPin *P : N->Pins)
-	}
+		{
-	if (!Found)
+			UWingServer::Printf(TEXT(" %s"), *WingUtils::FormatName(P));
-	{
+		}
-		UWingServer::Printf(TEXT("ERROR: Pin '%s' not found on node %s\n"),
+		UWingServer::Printf(TEXT("\n"));
-				*Value, *WingUtils::FormatName(N));
 		return SetError();
 	}
-
 	SetPin(Found);
 	return *this;
 }
@@ -301,11 +322,25 @@ WingFetcher& WingFetcher::Component(const FString& Value)
 
 	UBlueprint* BP = ::Cast<UBlueprint>(Obj);
 	if (!BP)
-		return TypeMismatch(TEXT("component"), TEXT("Blueprint"));
+	{
+		TypeMismatch(TEXT("component"), TEXT("Blueprint"));
+		PrintPathExplanation();
+		return SetError();
+	}
 
 	TArray<FWingActorComponent> AllComponents = FWingActorComponent::GetAll(BP);
-	FWingActorComponent* Comp = WingUtils::FindExactlyOneNamed(Value, AllComponents, TEXT("Component"));
+	FWingActorComponent* Comp = WingUtils::FindExactlyOneNamed(Value, AllComponents, TEXT("component"));
-	if (!Comp) return SetError();
+	if (!Comp)
+	{
+		UWingServer::Printf(TEXT("Components that exist in the blueprint:"));
+		for (const FWingActorComponent &C : AllComponents)
+		{
+			UWingServer::Printf(TEXT(" %s"), *WingUtils::FormatName(C));
+		}
+		UWingServer::Printf(TEXT("\n"));
+		return SetError();
+	}
+
 	if (!Comp->IsOwnedBy(BP))
 	{
 		UWingServer::Printf(TEXT("ERROR: Component '%s' belongs to %s, to edit it, you must go through that blueprint.\n"),
@@ -322,13 +357,25 @@ WingFetcher& WingFetcher::Widget(const FString& Value)
 
 	UWidgetBlueprint* WidgetBP = ::Cast<UWidgetBlueprint>(Obj);
 	if (!WidgetBP)
-		return TypeMismatch(TEXT("widget"), TEXT("WidgetBlueprint"));
+	{
+		TypeMismatch(TEXT("widget"), TEXT("WidgetBlueprint"));
+		PrintPathExplanation();
+		return SetError();
+	}
 
 	TArray<UWidget*> AllWidgets;
 	WidgetBP->WidgetTree->GetAllWidgets(AllWidgets);
-	UWidget* Found = WingUtils::FindExactlyOneNamed(Value, AllWidgets, TEXT("Widget"));
+	UWidget* Found = WingUtils::FindExactlyOneNamed(Value, AllWidgets, TEXT("widget"));
-	if (!Found) return SetError();
+	if (!Found)
-
+	{
+		UWingServer::Printf(TEXT("Widgets that exist in the blueprint:"));
+		for (const UWidget *W : AllWidgets)
+		{
+			UWingServer::Printf(TEXT(" %s"), *WingUtils::FormatName(W));
+		}
+		UWingServer::Printf(TEXT("\n"));
+		return SetError();
+	}
 	SetObj(Found);
 	return *this;
 }
@@ -339,7 +386,11 @@ WingFetcher& WingFetcher::LevelBlueprint(const FString& Value)
 
 	UWorld* World = ::Cast<UWorld>(Obj);
 	if (!World)
-		return TypeMismatch(TEXT("levelblueprint"), TEXT("World"));
+	{
+		TypeMismatch(TEXT("levelblueprint"), TEXT("world"));
+		PrintPathExplanation();
+		return SetError();
+	}
 
 	if (!World->PersistentLevel)
 	{

Plugins/UEWingman/Source/UEWingman/Private/WingUtils.cpp

@@ -459,7 +459,6 @@ TArray<UEdGraph*> WingUtils::AllGraphs(UBlueprint* BP)
 	return Graphs;
 }
 
-
 TArray<UEdGraphNode*> WingUtils::AllNodes(UBlueprint* BP)
 {
 	TArray<UEdGraphNode*> Nodes;
@@ -468,6 +467,13 @@ TArray<UEdGraphNode*> WingUtils::AllNodes(UBlueprint* BP)
 	return Nodes;
 }
 
+TArray<UEdGraphNode*> WingUtils::AllNodes(UEdGraph *Graph)
+{
+	TArray<UEdGraphNode*> Result;
+	Result.Append(Graph->Nodes);
+	return Result;
+}
+
 // ============================================================
 // Material helpers
 // ============================================================

Plugins/UEWingman/Source/UEWingman/Public/WingFetcher.h

@@ -37,6 +37,9 @@ struct FWalker;
 class WingFetcher
 {
 public:	
+	// Print a general explanation of what paths look like.
+	static void PrintPathExplanation();
+
 	// Walk a path from an asset to an object
 	// within that asset.  If you call walk a
 	// second time, it will walk additional steps.

Plugins/UEWingman/Source/UEWingman/Public/WingUtils.h

@@ -207,23 +207,8 @@ public:
 	// ----- Blueprint helpers -----
 	static TArray<UEdGraph*> AllGraphs(UBlueprint* BP);
 	static TArray<UEdGraphNode*> AllNodes(UBlueprint* BP);
-	template<class T> static TArray<T*> AllNodes(UBlueprint* BP)
+	static TArray<UEdGraphNode*> AllNodes(UEdGraph *Graph);
-	{
+
-		TArray<T*> Result;
-		for (UEdGraph* Graph : AllGraphs(BP))
-			for (UEdGraphNode* Node : Graph->Nodes)
-				if (T* Typed = Cast<T>(Node))
-					Result.Add(Typed);
-		return Result;
-	}
-	template<class T> static TArray<T*> AllNodes(UEdGraph* Graph)
-	{
-		TArray<T*> Result;
-		for (UEdGraphNode* Node : Graph->Nodes)
-			if (T* Typed = Cast<T>(Node))
-				Result.Add(Typed);
-		return Result;
-	}
 
 	// ----- Material helpers -----
 	static void EnsureMaterialGraph(UMaterial* Material);