Agents doing good job of improving handlers.

This commit is contained in:
2026-03-10 03:02:41 -04:00
parent 519933d532
commit 7ff4d0bccd
14 changed files with 537 additions and 189 deletions

View File

@@ -0,0 +1,117 @@
# Your Goals
We want to do several things. I'll give you
the concise list of goals, then I'll circle back
and give you the coding standard we're trying
to achieve.
- Convert these handlers to use plain-text output
wherever it is practical.
- MCPAssetFinder and MCPFetcher are powerful tools
to retrieve assets and objects. We want to migrate
the code to use these over hand-rolled solutions.
- Get rid of ad-hoc code to calculate object names,
which inevitably results in inconsistent naming.
- To make some of these handlers process batches,
rather than single objects, where it makes sense
to do so.
- To reduce the amount of code that's just passing
error messages around.
- To get rid of UE_LOG calls.
# Our Coding Standards
This is what we're trying to achieve with these goals.
* When searching for uassets, the tool MCPAssetFinder is
often the most precise, concise, and efficient tool.
Please study this API before starting. Using this
tool gives more reliable and more concise code than
hand-rolling code to obtain assets.
* Class MCPFetcher can precisely and easily retrieve objects
of all different kinds using clear, specific identifiers.
It is the best tool when you need the caller to specify a
blueprint, or a graph, or a pin - you name it. It relieves
you of the burden of digging around in unreal's data
structures. It also enforces a naming consistency
across handlers. Please study this API.
* To get the name of an object, there are tons of methods
you could call: pin->GetName(), pin->NodeGuid.ToString(),
node->GetTitle()... there are *so many* ways to get a
readable label. All of those ways are *NOT ALLOWED*. The
only valid way to get a name for an object is using
MCPUtils::FormatName(obj). This is to enforce consistent
naming. FormatName has been designed to produce very
clear, very readable, and highly unique identifiers.
More importantly, if one tool outputs a name, a different
tool will accept that name, because both tools use the
same naming scheme.
* To check whether a string matches an object's name,
there's only one valid way to do that as well: using
bool MCPUtils::Identifies(Name, Obj). Don't compare
the name yourself: you might get the case-sensitivity
wrong, you might forget to ignore trailing whitespace,
there are too many things that could go wrong. We've
provided MCPUtils::Identifies, which does it right
every time, for every kind of object.
* Concise output is better. The output of these handlers
will primarily be consumed by LLMs with finite context
windows. Plain-text handlers beat json handlers for
minimizing tokens: read the API for MCPHandler.h to learn
how to create a plain text handler. Avoid sending
information the caller didn't ask for. Don't echo the
command parameters: the caller knows what parameters he
sent. Don't make things unnecessarily verbose:
"type=int varname=x" can be shortened to "int x."
Generate output in a form that *you* would want to
consume.
* It's really important to send good error messages back to
the caller. Every handler has a second parameter, the
"output device", which is used to send data back to the
caller. The "output device" can either be a stringbuilder,
or a json tree. Our two core tools, MCPFetcher and
MCPAssetFinder, have powerful error handling built in: if
you give them a reference to the output device, they
will send error messages directly back to the caller
without your intervention. This is bulletproof.
Study class MCPErrorCallback to see how this works: any
function that takes MCPErrorCallback can put errors
directly into an "output device".
* If the output device of your handler is a shared ptr
to a json tree, you might see code that passes &*Json to
an MCPErrorCallback. That's legacy: it used to be necessary
to convert the shared pointer into a regular pointer.
Now the MCPErrorCallback can accept the shared pointer
directly.
* It's good for handlers to do operations in batches,
where possible. SpawnNodes is better than SpawnNode.
When an LLM calls into an MCP, it often takes 15 to 30
seconds. It's *important* that ConnectPins can do batches,
because building a graph can involve connecting dozens
of pins.
* It is traditional to use UE_LOG in unreal code, but it
really doesn't work for us: you see, the LLM invoking the
MCP can't see the log messages, so what's the point?
Better to report problems via the response. Please remove
UE_LOG calls in handlers.
* In addition to refactoring some handlers, we'd like you
to make some notes in a markdown file. The markdown
file should have the name UMCPHandler_XXX.Notes.md.
Tell us what you think is good about the handler, what
needs more work, and areas where you weren't entirely
confident about what to do, so you acted conservatively.
If there's more to do, we'd like to know.

View File

@@ -0,0 +1,31 @@
# UMCPHandler_DeleteAsset Refactoring Notes
## What Changed
- **Converted from JSON output to plain-text output.** The handler now overrides `Handle(FStringBuilderBase&)` instead of `Handle(FJsonObject*)`. This produces more concise, LLM-friendly output.
- **Removed all UE_LOG calls.** There were two UE_LOG calls logging the force-unload and delete operations. These are gone; all relevant information goes through the response instead.
- **Simplified the reference reporting.** The old code built separate JSON arrays for "live" vs "stale" referencers with counts for each. The new code prints a simple list with a "(loaded)" or "(on-disk only)" annotation per entry. This is much more concise while conveying the same information.
- **Removed unnecessary JSON fields from the response.** The old code echoed back `assetPath`, `filename`, `referencerCount`, `liveReferencerCount`, `staleReferencerCount`, and `suggestion` as separate JSON fields. The new code reports only what the caller needs to know.
- **Removed unused includes.** Dropped `Engine/Blueprint.h`, `Kismet2/BlueprintEditorUtils.h`, `UObject/SavePackage.h`, `AssetToolsModule.h`, `IAssetTools.h`, and `FileHelpers.h` which were not needed by this handler.
- **Cleaned up control flow.** The old code set `bDeleted` and then continued to set JSON fields regardless of success/failure. The new code returns early on failure with a clear error message.
## What's Good About This Handler
- The core logic is solid: it checks for the file on disk, checks references, handles force-unloading properly (clearing flags, resetting loaders, running GC), and rescans the asset registry after deletion.
- The force-unload sequence is careful and thorough — it handles package objects, flags, loaders, and garbage collection in the right order.
- The reference check with self-reference filtering is a good safeguard.
## What Might Need More Work
- **MCPAssetFinder not used.** I considered using `MCPAssets` to verify the asset exists, but this handler operates at the package/file level rather than searching by name or type. The handler needs the on-disk filename (via `LongPackageNameToFilename`) which MCPAssetFinder doesn't provide. Using MCPAssetFinder here would add an unnecessary asset registry query on top of the direct file check. I left the direct file-existence check in place.
- **No batching.** The handler deletes one asset at a time. If bulk deletion is a common use case, it could be extended to accept an array of paths. I left this as-is since the coding standards say to batch "where it makes sense" and bulk asset deletion seems like a less common operation where doing it one at a time is safer.
- **The reference check uses raw FName comparison.** The `Referencers.RemoveAll` lambda compares `Ref.ToString() == AssetPath` which relies on exact string equality. This works for the self-reference filtering case but could theoretically miss case differences. I preserved the existing behavior.

View File

@@ -4,15 +4,9 @@
#include "MCPHandler.h" #include "MCPHandler.h"
#include "MCPAssetFinder.h" #include "MCPAssetFinder.h"
#include "MCPUtils.h" #include "MCPUtils.h"
#include "Engine/Blueprint.h"
#include "Kismet2/BlueprintEditorUtils.h"
#include "UObject/SavePackage.h"
#include "Misc/PackageName.h" #include "Misc/PackageName.h"
#include "AssetRegistry/AssetRegistryModule.h" #include "AssetRegistry/AssetRegistryModule.h"
#include "AssetRegistry/IAssetRegistry.h" #include "AssetRegistry/IAssetRegistry.h"
#include "AssetToolsModule.h"
#include "IAssetTools.h"
#include "FileHelpers.h"
#include "HAL/FileManager.h" #include "HAL/FileManager.h"
#include "UMCPHandler_DeleteAsset.generated.h" #include "UMCPHandler_DeleteAsset.generated.h"
@@ -27,7 +21,7 @@ class UMCPHandler_DeleteAsset : public UObject, public IMCPHandler
GENERATED_BODY() GENERATED_BODY()
public: public:
UPROPERTY(meta=(Description="Package path of the asset to delete")) UPROPERTY(meta=(Description="Package path of the asset to delete, e.g. /Game/Foo/Bar"))
FString AssetPath; FString AssetPath;
UPROPERTY(meta=(Optional, Description="If true, skip reference check and force delete")) UPROPERTY(meta=(Optional, Description="If true, skip reference check and force delete"))
@@ -39,17 +33,17 @@ public:
"Use force=true to skip the reference check."); "Use force=true to skip the reference check.");
} }
virtual void Handle(const FJsonObject* Json, FJsonObject* Result) override virtual void Handle(const FJsonObject* Json, FStringBuilderBase& Result) override
{ {
// Verify the asset file exists on disk
// Check if asset file exists on disk
FString PackageFilename = FPackageName::LongPackageNameToFilename( FString PackageFilename = FPackageName::LongPackageNameToFilename(
AssetPath, FPackageName::GetAssetPackageExtension()); AssetPath, FPackageName::GetAssetPackageExtension());
PackageFilename = FPaths::ConvertRelativePathToFull(PackageFilename); PackageFilename = FPaths::ConvertRelativePathToFull(PackageFilename);
if (!IFileManager::Get().FileExists(*PackageFilename)) if (!IFileManager::Get().FileExists(*PackageFilename))
{ {
return MCPUtils::MakeErrorJson(Result, FString::Printf(TEXT("Asset file not found on disk: %s"), *PackageFilename)); Result.Appendf(TEXT("ERROR: Asset file not found on disk: %s\n"), *PackageFilename);
return;
} }
// Check references // Check references
@@ -62,55 +56,31 @@ public:
return Ref.ToString() == AssetPath; return Ref.ToString() == AssetPath;
}); });
if ((Referencers.Num() > 0) && !Force) if (Referencers.Num() > 0 && !Force)
{ {
// Classify references as "live" (loaded in memory) vs "stale" (only on disk) Result.Appendf(TEXT("ERROR: Asset is still referenced by %d package(s):\n"), Referencers.Num());
TArray<TSharedPtr<FJsonValue>> LiveRefs;
TArray<TSharedPtr<FJsonValue>> StaleRefs;
for (const FName& Ref : Referencers) for (const FName& Ref : Referencers)
{ {
FString RefStr = Ref.ToString(); FString RefStr = Ref.ToString();
UPackage* RefPackage = FindPackage(nullptr, *RefStr); UPackage* RefPackage = FindPackage(nullptr, *RefStr);
if (RefPackage) Result.Appendf(TEXT(" %s%s\n"), *RefStr,
{ RefPackage ? TEXT(" (loaded)") : TEXT(" (on-disk only)"));
LiveRefs.Add(MakeShared<FJsonValueString>(RefStr));
}
else
{
StaleRefs.Add(MakeShared<FJsonValueString>(RefStr));
}
} }
Result.Append(TEXT("Use force=true to skip the reference check.\n"));
MCPUtils::MakeErrorJson(Result, TEXT("Asset is still referenced. Remove all references first."));
Result->SetStringField(TEXT("assetPath"), AssetPath);
Result->SetNumberField(TEXT("referencerCount"), Referencers.Num());
Result->SetNumberField(TEXT("liveReferencerCount"), LiveRefs.Num());
Result->SetArrayField(TEXT("liveReferencers"), LiveRefs);
Result->SetNumberField(TEXT("staleReferencerCount"), StaleRefs.Num());
Result->SetArrayField(TEXT("staleReferencers"), StaleRefs);
Result->SetStringField(TEXT("suggestion"),
StaleRefs.Num() > 0
? TEXT("Some references may be stale. Consider force=true to skip the reference check, or use change_variable_type to migrate references first.")
: TEXT("All references are live. Migrate with change_variable_type or replace_function_calls before deleting."));
return; return;
} }
// Force delete: unload the package from memory first // Force delete: unload the package from memory first
TArray<TSharedPtr<FJsonValue>> RefWarnings; if (Force && Referencers.Num() > 0)
{
Result.Appendf(TEXT("WARNING: Force-deleting despite %d referencer(s).\n"), Referencers.Num());
}
if (Force) if (Force)
{ {
// Collect reference warnings when force-deleting with existing references
for (const FName& Ref : Referencers)
{
RefWarnings.Add(MakeShared<FJsonValueString>(
FString::Printf(TEXT("Warning: '%s' still references this asset"), *Ref.ToString())));
}
UPackage* Package = FindPackage(nullptr, *AssetPath); UPackage* Package = FindPackage(nullptr, *AssetPath);
if (Package) if (Package)
{ {
UE_LOG(LogTemp, Display, TEXT("BlueprintMCP: Force-unloading package '%s' from memory"), *AssetPath);
// Collect all objects in this package // Collect all objects in this package
TArray<UObject*> ObjectsInPackage; TArray<UObject*> ObjectsInPackage;
GetObjectsWithPackage(Package, ObjectsInPackage); GetObjectsWithPackage(Package, ObjectsInPackage);
@@ -134,35 +104,24 @@ public:
} }
} }
UE_LOG(LogTemp, Display, TEXT("BlueprintMCP: Deleting asset '%s' (%s)%s"),
*AssetPath, *PackageFilename, Force ? TEXT(" [FORCE]") : TEXT(""));
// Delete the file on disk // Delete the file on disk
bool bDeleted = IFileManager::Get().Delete(*PackageFilename, false, true); bool bDeleted = IFileManager::Get().Delete(*PackageFilename, false, true);
if (bDeleted)
{
// Trigger an asset registry rescan so it notices the deletion
TArray<FString> PathsToScan;
int32 LastSlash;
if (AssetPath.FindLastChar(TEXT('/'), LastSlash))
{
PathsToScan.Add(AssetPath.Left(LastSlash));
}
if (PathsToScan.Num() > 0)
{
Registry.ScanPathsSynchronous(PathsToScan, true);
}
}
Result->SetStringField(TEXT("filename"), PackageFilename);
if (!bDeleted) if (!bDeleted)
{ {
MCPUtils::MakeErrorJson(Result, TEXT("Failed to delete file from disk")); Result.Appendf(TEXT("ERROR: Failed to delete file from disk: %s\n"), *PackageFilename);
return;
} }
if (RefWarnings.Num() > 0)
// Trigger an asset registry rescan so it notices the deletion
FString PackageDir;
int32 LastSlash;
if (AssetPath.FindLastChar(TEXT('/'), LastSlash))
{ {
Result->SetArrayField(TEXT("warnings"), RefWarnings); PackageDir = AssetPath.Left(LastSlash);
Registry.ScanPathsSynchronous({PackageDir}, true);
} }
Result.Appendf(TEXT("Deleted %s\n"), *AssetPath);
} }
}; };

View File

@@ -0,0 +1,32 @@
# UMCPHandler_FindAssetReferences - Refactoring Notes
## What Changed
- **Switched from JSON to plain-text output.** The old handler used the `Handle(Json, FJsonObject*)` signature and built a JSON tree with counts and arrays. The new handler uses `Handle(Json, FStringBuilderBase&)` and emits one line per referencer in `ClassName /Package/Path` format, matching the pattern used by SearchAssets.
- **Removed the all-blueprints scan.** The old code loaded the entire MCPAssets<UBlueprint> index just to classify referencers as "blueprint" vs "other." This was expensive and the categorization wasn't particularly useful to an LLM caller. The new code instead looks up each referencer's asset class from the asset registry, which is cheap and gives more specific type information (e.g., "WidgetBlueprint" vs "AnimBlueprint" vs "Material" rather than just "blueprint" or "other").
- **Added asset existence validation.** The old handler silently returned empty results if the asset path was wrong. The new handler checks the asset registry and returns a clear error if the asset doesn't exist.
- **Removed redundant count fields.** The old handler sent totalReferencers, blueprintReferencerCount, and otherReferencerCount. The LLM can count lines if it needs to. This keeps the output concise.
- **Removed unnecessary includes.** The old handler included many K2Node headers, EdGraph headers, Blueprint headers, etc. None of these were actually used by this handler -- they were likely copy-paste artifacts.
- **No UE_LOG calls** -- the old handler didn't have any, so nothing to remove there.
## What's Good About This Handler
- It's simple and focused: one asset path in, list of referencers out.
- The plain-text output is concise and easy for an LLM to parse.
- Each line includes the asset class name, which tells the caller what kind of asset each referencer is without requiring a separate lookup.
- Error handling is clear: bad asset path gets a descriptive error.
## What Might Need More Work
- **MCPFetcher/MCPAssetFinder not used for input validation.** The input is a raw package path passed directly to the asset registry. I considered using MCPAssetFinder with `Exact()` to resolve the asset, but `GetReferencers` takes an FName package path, and MCPAssetFinder is designed more for searching/filtering than for validating a known exact path. Using the asset registry's `GetAssetByObjectPath` directly seemed more appropriate here. If there's a preferred pattern for validating package paths, this could be updated.
- **No batching.** This handler processes a single asset path. It could be extended to accept multiple paths, but it wasn't clear whether that would be useful -- the caller can just invoke it multiple times if needed, and reference lookups are fast.
- **GetReferencers returns package-level dependencies.** This is an Unreal asset registry limitation: it shows that package A references package B, but doesn't pinpoint which node or property holds the reference. A more detailed handler could load the blueprints and walk their graphs to find exact usage locations, but that would be a significantly different tool.
- **The FSoftObjectPath constructor.** I used `FSoftObjectPath(AssetPath)` for the existence check. This works for standard `/Game/...` paths but I'm not 100% certain it handles all edge cases (e.g., paths with subobjects). If validation fails for valid paths, this would be the place to investigate.

View File

@@ -2,23 +2,7 @@
#include "CoreMinimal.h" #include "CoreMinimal.h"
#include "MCPHandler.h" #include "MCPHandler.h"
#include "MCPAssetFinder.h"
#include "MCPUtils.h" #include "MCPUtils.h"
#include "Engine/Blueprint.h"
#include "Engine/World.h"
#include "Engine/Level.h"
#include "Engine/LevelScriptBlueprint.h"
#include "EdGraph/EdGraph.h"
#include "EdGraph/EdGraphNode.h"
#include "K2Node_CallFunction.h"
#include "K2Node_Event.h"
#include "K2Node_CustomEvent.h"
#include "K2Node_VariableGet.h"
#include "K2Node_VariableSet.h"
#include "K2Node_BreakStruct.h"
#include "K2Node_MakeStruct.h"
#include "K2Node_FunctionEntry.h"
#include "K2Node_EditablePinBase.h"
#include "AssetRegistry/IAssetRegistry.h" #include "AssetRegistry/IAssetRegistry.h"
#include "UMCPHandler_FindAssetReferences.generated.h" #include "UMCPHandler_FindAssetReferences.generated.h"
@@ -27,59 +11,57 @@
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// ============================================================
// HandleFindReferences — find all Blueprints referencing an asset
// ============================================================
UCLASS() UCLASS()
class UMCPHandler_FindAssetReferences : public UObject, public IMCPHandler class UMCPHandler_FindAssetReferences : public UObject, public IMCPHandler
{ {
GENERATED_BODY() GENERATED_BODY()
public: public:
UPROPERTY(meta=(Description="Asset package path to find references for")) UPROPERTY(meta=(Description="Asset package path to find references for, e.g. /Game/Tangibles/TAN_Tree"))
FString AssetPath; FString AssetPath;
virtual FString GetDescription() const override virtual FString GetDescription() const override
{ {
return TEXT("Find all assets that reference a given asset, categorized into Blueprint and non-Blueprint referencers."); return TEXT("Find all assets that reference a given asset.");
} }
virtual void Handle(const FJsonObject* Json, FJsonObject* Result) override virtual void Handle(const FJsonObject* Json, FStringBuilderBase& Result) override
{ {
IAssetRegistry& Registry = *IAssetRegistry::Get(); IAssetRegistry& Registry = *IAssetRegistry::Get();
// Verify the asset exists
FAssetData AssetData = Registry.GetAssetByObjectPath(FSoftObjectPath(AssetPath));
if (!AssetData.IsValid())
{
Result.Appendf(TEXT("ERROR: Asset not found: %s\n"), *AssetPath);
return;
}
TArray<FName> Referencers; TArray<FName> Referencers;
Registry.GetReferencers(FName(*AssetPath), Referencers); Registry.GetReferencers(FName(*AssetPath), Referencers);
// Build set of known Blueprint package names for filtering if (Referencers.Num() == 0)
MCPAssets<UBlueprint> AllBlueprints;
AllBlueprints.Info();
TSet<FString> BlueprintPackages;
for (const FAssetData& Asset : AllBlueprints.AllData())
{ {
BlueprintPackages.Add(Asset.PackageName.ToString()); Result.Append(TEXT("No referencers found.\n"));
return;
} }
TArray<TSharedPtr<FJsonValue>> BPRefs; // Classify referencers by looking up their asset class
TArray<TSharedPtr<FJsonValue>> OtherRefs;
for (const FName& Ref : Referencers) for (const FName& Ref : Referencers)
{ {
FString RefStr = Ref.ToString(); FString RefStr = Ref.ToString();
if (BlueprintPackages.Contains(RefStr)) TArray<FAssetData> RefAssets;
Registry.GetAssetsByPackageName(Ref, RefAssets);
if (RefAssets.Num() > 0)
{ {
BPRefs.Add(MakeShared<FJsonValueString>(RefStr)); Result.Appendf(TEXT("%s %s\n"),
*MCPUtils::FormatName(RefAssets[0].GetClass()),
*RefStr);
} }
else else
{ {
OtherRefs.Add(MakeShared<FJsonValueString>(RefStr)); Result.Appendf(TEXT("Unknown %s\n"), *RefStr);
} }
} }
Result->SetNumberField(TEXT("totalReferencers"), Referencers.Num());
Result->SetNumberField(TEXT("blueprintReferencerCount"), BPRefs.Num());
Result->SetArrayField(TEXT("blueprintReferencers"), BPRefs);
Result->SetNumberField(TEXT("otherReferencerCount"), OtherRefs.Num());
Result->SetArrayField(TEXT("otherReferencers"), OtherRefs);
} }
}; };

View File

@@ -0,0 +1,30 @@
# UMCPHandler_GetNodeComment - Refactoring Notes
## What Changed
- **Switched from JSON to plain-text output.** The handler now overrides `Handle(Json, FStringBuilderBase&)` instead of `Handle(Json, FJsonObject*)`. The output is three simple lines: node name, comment text, and bubble visibility.
- **MCPFetcher now receives the FStringBuilderBase.** Previously it received `Result` (a `FJsonObject*`); now it receives the string builder, so errors go directly into the plain-text response.
- **Added MCPUtils::FormatName for the node.** The output now identifies the node using the standard naming scheme, so the caller can confirm which node was queried.
- **Improved the example path in the description.** Changed from `/Game/Foo,node:MyNode` to `/Game/Foo,graph:EventGraph,node:MyNode` which is a more realistic path (nodes live inside graphs).
- **Removed the leading blank line** inside the Handle method (minor cleanup matching the style of the example handlers).
## What's Good
- The handler is simple and focused - it does one thing well.
- It already used MCPFetcher for object resolution, which is correct.
- The plain-text output is minimal and token-efficient for LLM consumption.
- No UE_LOG calls were present, so none needed removal.
## Areas of Uncertainty
- **Whether to include the node name in output.** The coding standards say "don't echo the command parameters," but the node name shown via FormatName may differ from what the caller sent (e.g. they sent a partial match). I included it so the caller can confirm the resolved node. This could be removed if considered too verbose.
- **Batching.** The coding standards encourage batch operations. This handler could be extended to accept multiple node paths and return comments for all of them. I did not make that change since it would alter the tool's interface and wasn't explicitly requested, but it's worth considering.
## No Further Issues
The handler is straightforward and the refactoring was clean. No other concerns.

View File

@@ -18,7 +18,7 @@ class UMCPHandler_GetNodeComment : public UObject, public IMCPHandler
GENERATED_BODY() GENERATED_BODY()
public: public:
UPROPERTY(meta=(Description="Path to the node, e.g. /Game/Foo,node:MyNode")) UPROPERTY(meta=(Description="Path to the node, e.g. /Game/Foo,graph:EventGraph,node:MyNode"))
FString Node; FString Node;
virtual FString GetDescription() const override virtual FString GetDescription() const override
@@ -26,14 +26,14 @@ public:
return TEXT("Get the comment text and bubble visibility of a node."); return TEXT("Get the comment text and bubble visibility of a node.");
} }
virtual void Handle(const FJsonObject* Json, FJsonObject* Result) override virtual void Handle(const FJsonObject* Json, FStringBuilderBase& Result) override
{ {
MCPFetcher F(Result); MCPFetcher F(Result);
UEdGraphNode* FoundNode = F.Walk(Node).Cast<UEdGraphNode>(); UEdGraphNode* FoundNode = F.Walk(Node).Cast<UEdGraphNode>();
if (!FoundNode) return; if (!FoundNode) return;
Result->SetStringField(TEXT("comment"), FoundNode->NodeComment); Result.Appendf(TEXT("Node: %s\n"), *MCPUtils::FormatName(FoundNode));
Result->SetBoolField(TEXT("commentBubbleVisible"), FoundNode->bCommentBubbleVisible); Result.Appendf(TEXT("Comment: %s\n"), *FoundNode->NodeComment);
Result.Appendf(TEXT("BubbleVisible: %s\n"), FoundNode->bCommentBubbleVisible ? TEXT("true") : TEXT("false"));
} }
}; };

View File

@@ -0,0 +1,55 @@
# UMCPHandler_ListEventDispatchers Refactoring Notes
## What Changed
- **Switched from JSON to plain-text output.** The handler now uses
`Handle(Json, FStringBuilderBase& Result)` instead of the JSON
overload. Output is one line per dispatcher in the form
`DispatcherName(Type Param, Type Param)`.
- **Switched from MCPAssets to MCPFetcher for blueprint lookup.**
The old code used `MCPAssets<UBlueprint>` with Exact/ENone/ETwo,
which is designed for asset searches. Since we have a single
path to resolve, MCPFetcher is the right tool. It also gives
us free support for walker segments in the path.
- **Renamed parameter from `Blueprint` to `Path`.** This is
consistent with how other fetcher-based handlers name their
parameter (see DumpGraphs).
- **Used MCPUtils::FormatPinType instead of hand-rolled type
formatting.** The old code built type strings from
`PinCategory` and `PinSubCategoryObject` manually. FormatPinType
handles all the edge cases (containers, references, etc.)
and is the canonical way to format types.
- **Removed the count field and "dispatchers" wrapper.** Plain-text
output doesn't need these. The caller can count the lines.
- **Added a "No event dispatchers found" message** for the empty
case, so the caller gets clear feedback.
## What's Good
- The core logic (GetDelegateNameList, signature graph lookup,
iterating UserDefinedPins) is correct and well-structured.
- No UE_LOG calls were present -- nothing to remove.
- The nesting depth is acceptable.
## Uncertainties / Potential Issues
- **FormatName not used for dispatcher names.** There is no
`FormatName` overload for delegates/event dispatchers (they're
just `FName` values from `GetDelegateNameList`). I used
`DelegateName.ToString()` directly. If a FormatName overload
for delegates is added later, this should be updated.
- **Parameter name formatting.** Similarly, `PinInfo->PinName`
is output via `.ToString()`. There's no FormatName overload
for `FUserPinInfo`. This seems fine since these are
user-defined parameter names, not graph objects.
- **Iteration order.** `TSet<FName>` has no guaranteed order.
If deterministic output ordering matters, the names should be
sorted. I left it as-is since the original code also used
unsorted iteration.

View File

@@ -2,7 +2,7 @@
#include "CoreMinimal.h" #include "CoreMinimal.h"
#include "MCPHandler.h" #include "MCPHandler.h"
#include "MCPAssetFinder.h" #include "MCPFetcher.h"
#include "MCPUtils.h" #include "MCPUtils.h"
#include "Engine/Blueprint.h" #include "Engine/Blueprint.h"
#include "EdGraph/EdGraph.h" #include "EdGraph/EdGraph.h"
@@ -23,64 +23,52 @@ class UMCPHandler_ListEventDispatchers : public UObject, public IMCPHandler
GENERATED_BODY() GENERATED_BODY()
public: public:
UPROPERTY(meta=(Description="Blueprint name or package path")) UPROPERTY(meta=(Description="Path to a blueprint, e.g. /Game/Foo/MyBlueprint"))
FString Blueprint; FString Path;
virtual FString GetDescription() const override virtual FString GetDescription() const override
{ {
return TEXT("List all event dispatchers on a Blueprint, including their parameter signatures."); return TEXT("List all event dispatchers on a Blueprint, including their parameter signatures.");
} }
virtual void Handle(const FJsonObject* Json, FJsonObject* Result) override virtual void Handle(const FJsonObject* Json, FStringBuilderBase& Result) override
{ {
MCPAssets<UBlueprint> Assets; MCPFetcher F(Result);
if (!Assets.Exact(Blueprint).Errors(Result).ENone().ETwo().Load()) return; UBlueprint* BP = F.Walk(Path).Cast<UBlueprint>();
UBlueprint* BP = Assets.Object(); if (!BP) return;
TSet<FName> DelegateNameSet; TSet<FName> DelegateNameSet;
FBlueprintEditorUtils::GetDelegateNameList(BP, DelegateNameSet); FBlueprintEditorUtils::GetDelegateNameList(BP, DelegateNameSet);
TArray<TSharedPtr<FJsonValue>> DispatchersArr;
for (const FName& DelegateName : DelegateNameSet) for (const FName& DelegateName : DelegateNameSet)
{ {
TSharedRef<FJsonObject> DispObj = MakeShared<FJsonObject>(); Result.Appendf(TEXT("%s("), *DelegateName.ToString());
DispObj->SetStringField(TEXT("name"), DelegateName.ToString());
// Get parameter info from the signature graph
TArray<TSharedPtr<FJsonValue>> ParamsArr;
UEdGraph* SigGraph = FBlueprintEditorUtils::GetDelegateSignatureGraphByName(BP, DelegateName); UEdGraph* SigGraph = FBlueprintEditorUtils::GetDelegateSignatureGraphByName(BP, DelegateName);
if (SigGraph) if (SigGraph)
{ {
bool bFirst = true;
for (UK2Node_FunctionEntry* FE : MCPUtils::AllNodes<UK2Node_FunctionEntry>(SigGraph)) for (UK2Node_FunctionEntry* FE : MCPUtils::AllNodes<UK2Node_FunctionEntry>(SigGraph))
{ {
for (const TSharedPtr<FUserPinInfo>& PinInfo : FE->UserDefinedPins) for (const TSharedPtr<FUserPinInfo>& PinInfo : FE->UserDefinedPins)
{ {
if (!PinInfo.IsValid()) continue; if (!PinInfo.IsValid()) continue;
if (!bFirst) Result.Append(TEXT(", "));
TSharedRef<FJsonObject> ParamObj = MakeShared<FJsonObject>(); bFirst = false;
ParamObj->SetStringField(TEXT("name"), PinInfo->PinName.ToString()); Result.Appendf(TEXT("%s %s"),
*MCPUtils::FormatPinType(PinInfo->PinType),
// Build a human-readable type name from the pin type *PinInfo->PinName.ToString());
FString TypeStr = PinInfo->PinType.PinCategory.ToString();
if (PinInfo->PinType.PinSubCategoryObject.IsValid())
{
TypeStr = PinInfo->PinType.PinSubCategoryObject->GetName();
}
ParamObj->SetStringField(TEXT("type"), TypeStr);
ParamsArr.Add(MakeShared<FJsonValueObject>(ParamObj));
} }
break; // only need the first entry node break; // only need the first entry node
} }
} }
DispObj->SetArrayField(TEXT("parameters"), ParamsArr); Result.Append(TEXT(")\n"));
DispatchersArr.Add(MakeShared<FJsonValueObject>(DispObj));
} }
Result->SetNumberField(TEXT("count"), DispatchersArr.Num()); if (DelegateNameSet.Num() == 0)
Result->SetArrayField(TEXT("dispatchers"), DispatchersArr); {
Result.Append(TEXT("No event dispatchers found.\n"));
}
} }
}; };

View File

@@ -0,0 +1,24 @@
# UMCPHandler_RenameAsset Refactoring Notes
## What Changed
- **Switched to plain-text output.** The handler now overrides `Handle(... FStringBuilderBase&)` instead of the JSON version. The output is minimal: just the new path on success, or a clear error message on failure.
- **Removed UE_LOG calls.** Two UE_LOG calls were removed. Errors are reported via the output device.
- **Removed the empty `if (bSuccess) {}` block.** This was dead code.
- **Used `MCPUtils::SplitAssetPath`** instead of hand-rolled path splitting with `FindLastChar`. This is more consistent and handles edge cases (like empty asset names) that the original code did not.
- **Added `AllContent()`** to the MCPAssets query so it can find assets outside `/Game/` if needed.
- **Trimmed unused includes.** Removed includes for headers that were not needed (BlueprintEditorUtils, SavePackage, PackageName, AssetRegistryModule, FileHelpers, Engine/Blueprint.h).
- **Improved parameter descriptions** with examples to help the LLM caller understand the expected format.
## What's Good
- The handler is simple and focused — it does one thing well.
- MCPAssets is used correctly: `Exact()` match, `ENone()` + `ETwo()` error conditions, with errors routed directly to the output device.
- The logic for "just a name with no slash means rename in place" is preserved and useful.
## What Might Need More Work
- **No FormatName/Identifies usage.** This handler doesn't look up objects by display name — it works with exact package paths, which is the correct thing for asset rename. FormatName/Identifies are not applicable here since we're dealing with package paths, not display names of nodes/pins/graphs.
- **No batch support.** The handler renames one asset at a time. Batch renaming could be useful, but it's a less common operation than batch pin connections, so this may not be worth the added complexity.
- **Error detail from RenameAssets is lost.** `IAssetTools::RenameAssets` returns a bool but doesn't provide a detailed reason for failure. The error message is necessarily generic. I'm not sure if there's a way to get more detail from Unreal's rename API.
- **No validation of NewAssetName.** We don't check for illegal characters in the new name before calling RenameAssets. Unreal will reject bad names, but a more specific error message could be helpful.

View File

@@ -4,15 +4,8 @@
#include "MCPHandler.h" #include "MCPHandler.h"
#include "MCPAssetFinder.h" #include "MCPAssetFinder.h"
#include "MCPUtils.h" #include "MCPUtils.h"
#include "Engine/Blueprint.h"
#include "Kismet2/BlueprintEditorUtils.h"
#include "UObject/SavePackage.h"
#include "Misc/PackageName.h"
#include "AssetRegistry/AssetRegistryModule.h"
#include "AssetRegistry/IAssetRegistry.h"
#include "AssetToolsModule.h" #include "AssetToolsModule.h"
#include "IAssetTools.h" #include "IAssetTools.h"
#include "FileHelpers.h"
#include "UMCPHandler_RenameAsset.generated.h" #include "UMCPHandler_RenameAsset.generated.h"
@@ -26,10 +19,10 @@ class UMCPHandler_RenameAsset : public UObject, public IMCPHandler
GENERATED_BODY() GENERATED_BODY()
public: public:
UPROPERTY(meta=(Description="Current package path of the asset")) UPROPERTY(meta=(Description="Current package path of the asset, e.g. /Game/Widgets/WB_Hotkeys"))
FString AssetPath; FString AssetPath;
UPROPERTY(meta=(Description="New package path or new asset name")) UPROPERTY(meta=(Description="New package path (e.g. /Game/Widgets/WB_NewName) or just a new asset name (e.g. WB_NewName)"))
FString NewPath; FString NewPath;
virtual FString GetDescription() const override virtual FString GetDescription() const override
@@ -37,59 +30,41 @@ public:
return TEXT("Rename or move an asset with reference fixup."); return TEXT("Rename or move an asset with reference fixup.");
} }
virtual void Handle(const FJsonObject* Json, FJsonObject* Result) override virtual void Handle(const FJsonObject* Json, FStringBuilderBase& Result) override
{ {
// Load the asset
UE_LOG(LogTemp, Display, TEXT("BlueprintMCP: Renaming asset '%s' -> '%s'"), *AssetPath, *NewPath);
// Use FAssetToolsModule to perform the rename with reference fixup
FAssetToolsModule& AssetToolsModule = FModuleManager::LoadModuleChecked<FAssetToolsModule>("AssetTools");
IAssetTools& AssetTools = AssetToolsModule.Get();
// Build the source/dest arrays
TArray<FAssetRenameData> RenameData;
// We need to load the asset to get the object
MCPAssets<UObject> Assets; MCPAssets<UObject> Assets;
if (!Assets.Exact(AssetPath).Errors(Result).ENone().ETwo().Load()) return; if (!Assets.Exact(AssetPath).AllContent().Errors(Result).ENone().ETwo().Load()) return;
UObject* AssetObj = Assets.Object(); UObject* AssetObj = Assets.Object();
// Parse new path into package path and asset name // Parse new path into package path and asset name
FString NewPackagePath, NewAssetName; FString NewPackagePath, NewAssetName;
int32 LastSlash; if (!MCPUtils::SplitAssetPath(NewPath, NewPackagePath, NewAssetName))
if (NewPath.FindLastChar(TEXT('/'), LastSlash))
{ {
NewPackagePath = NewPath.Left(LastSlash); // No slash — just a new name, keep the same directory
NewAssetName = NewPath.Mid(LastSlash + 1); FString OldPackagePath, OldAssetName;
} if (!MCPUtils::SplitAssetPath(AssetPath, OldPackagePath, OldAssetName))
else
{
// If no slash, assume same directory with new name
FString OldPackagePath;
if (AssetPath.FindLastChar(TEXT('/'), LastSlash))
{ {
OldPackagePath = AssetPath.Left(LastSlash); Result.Appendf(TEXT("ERROR: Cannot determine directory from AssetPath '%s'\n"), *AssetPath);
return;
} }
NewPackagePath = OldPackagePath; NewPackagePath = OldPackagePath;
NewAssetName = NewPath; NewAssetName = NewPath;
} }
FAssetRenameData RenameEntry(AssetObj, NewPackagePath, NewAssetName); // Perform the rename with reference fixup
RenameData.Add(RenameEntry); FAssetToolsModule& AssetToolsModule = FModuleManager::LoadModuleChecked<FAssetToolsModule>("AssetTools");
IAssetTools& AssetTools = AssetToolsModule.Get();
bool bSuccess = AssetTools.RenameAssets(RenameData); TArray<FAssetRenameData> RenameData;
RenameData.Add(FAssetRenameData(AssetObj, NewPackagePath, NewAssetName));
if (bSuccess) if (!AssetTools.RenameAssets(RenameData))
{ {
Result.Append(TEXT("ERROR: Rename failed. The target path may be invalid or a conflicting asset may exist.\n"));
return;
} }
UE_LOG(LogTemp, Display, TEXT("BlueprintMCP: Rename %s"), bSuccess ? TEXT("succeeded") : TEXT("failed")); Result.Appendf(TEXT("Renamed to %s/%s\n"), *NewPackagePath, *NewAssetName);
Result->SetStringField(TEXT("newPackagePath"), NewPackagePath);
Result->SetStringField(TEXT("newAssetName"), NewAssetName);
if (!bSuccess)
{
MCPUtils::MakeErrorJson(Result, TEXT("Asset rename failed. The target path may be invalid or a conflicting asset may exist."));
}
} }
}; };

View File

@@ -64,7 +64,7 @@ public:
for (const FAssetData& Data : AllData) for (const FAssetData& Data : AllData)
{ {
Result.Appendf(TEXT("%s %s\n"), Result.Appendf(TEXT("%s %s\n"),
*Data.AssetClassPath.GetAssetName().ToString(), *MCPUtils::FormatName(Data.GetClass()),
*Data.PackageName.ToString()); *Data.PackageName.ToString());
} }

View File

@@ -69,10 +69,18 @@
#include "Materials/MaterialExpressionFunctionOutput.h" #include "Materials/MaterialExpressionFunctionOutput.h"
#include "Materials/MaterialExpressionMaterialFunctionCall.h" #include "Materials/MaterialExpressionMaterialFunctionCall.h"
#include "Materials/MaterialFunction.h" #include "Materials/MaterialFunction.h"
#include "Materials/MaterialInstanceConstant.h"
#include "MaterialGraph/MaterialGraph.h" #include "MaterialGraph/MaterialGraph.h"
#include "MaterialGraph/MaterialGraphNode.h" #include "MaterialGraph/MaterialGraphNode.h"
#include "MaterialGraph/MaterialGraphSchema.h" #include "MaterialGraph/MaterialGraphSchema.h"
// Mesh, animation, texture support
#include "Engine/StaticMesh.h"
#include "Engine/SkeletalMesh.h"
#include "Animation/AnimSequence.h"
#include "Animation/BlendSpace.h"
#include "Engine/Texture.h"
// SEH support (Windows only) — defined in BlueprintMCPServer.cpp // SEH support (Windows only) — defined in BlueprintMCPServer.cpp
#if PLATFORM_WINDOWS #if PLATFORM_WINDOWS
extern int32 TryCompileBlueprintSEH(UBlueprint* BP, EBlueprintCompileOptions Opts); extern int32 TryCompileBlueprintSEH(UBlueprint* BP, EBlueprintCompileOptions Opts);
@@ -189,11 +197,127 @@ FString MCPUtils::FormatName(const UClass *Class)
return Name; return Name;
} }
FString MCPUtils::FormatName(const UMaterial *Material)
{
return Material->GetPathName();
}
FString MCPUtils::FormatName(const UMaterialInstance *MaterialInstance)
{
return MaterialInstance->GetPathName();
}
FString MCPUtils::FormatName(const UMaterialFunction *MaterialFunction)
{
return MaterialFunction->GetPathName();
}
FString MCPUtils::FormatName(const UMaterialExpression *Expression)
{
FString Name = Expression->GetName();
SanitizeNameInPlace(Name);
return Name;
}
FString MCPUtils::FormatName(const UStaticMesh *Mesh)
{
return Mesh->GetPathName();
}
FString MCPUtils::FormatName(const USkeletalMesh *Mesh)
{
return Mesh->GetPathName();
}
FString MCPUtils::FormatName(const UAnimSequence *Anim)
{
return Anim->GetPathName();
}
FString MCPUtils::FormatName(const UBlendSpace *BlendSpace)
{
return BlendSpace->GetPathName();
}
FString MCPUtils::FormatName(const UTexture *Texture)
{
return Texture->GetPathName();
}
FString MCPUtils::FormatName(const UScriptStruct *Struct)
{
FString Name = Struct->GetName();
SanitizeNameInPlace(Name);
return Name;
}
FString MCPUtils::FormatName(const UEnum *Enum)
{
FString Name = Enum->GetName();
SanitizeNameInPlace(Name);
return Name;
}
bool MCPUtils::Identifies(const FString &Name, const UClass *Class) bool MCPUtils::Identifies(const FString &Name, const UClass *Class)
{ {
return FormatName(Class).Equals(Name, ESearchCase::IgnoreCase); return FormatName(Class).Equals(Name, ESearchCase::IgnoreCase);
} }
bool MCPUtils::Identifies(const FString &Name, const UMaterial *Material)
{
return FormatName(Material).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UMaterialInstance *MaterialInstance)
{
return FormatName(MaterialInstance).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UMaterialFunction *MaterialFunction)
{
return FormatName(MaterialFunction).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UMaterialExpression *Expression)
{
return FormatName(Expression).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UStaticMesh *Mesh)
{
return FormatName(Mesh).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const USkeletalMesh *Mesh)
{
return FormatName(Mesh).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UAnimSequence *Anim)
{
return FormatName(Anim).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UBlendSpace *BlendSpace)
{
return FormatName(BlendSpace).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UTexture *Texture)
{
return FormatName(Texture).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UScriptStruct *Struct)
{
return FormatName(Struct).Equals(Name, ESearchCase::IgnoreCase);
}
bool MCPUtils::Identifies(const FString &Name, const UEnum *Enum)
{
return FormatName(Enum).Equals(Name, ESearchCase::IgnoreCase);
}
// ============================================================ // ============================================================
// Identifies // Identifies
// ============================================================ // ============================================================

View File

@@ -9,6 +9,8 @@ class UEdGraph;
class UEdGraphNode; class UEdGraphNode;
class UEdGraphPin; class UEdGraphPin;
class UMaterial; class UMaterial;
class UMaterialInstance;
class UMaterialFunction;
class UMaterialExpression; class UMaterialExpression;
class UBlueprintNodeSpawner; class UBlueprintNodeSpawner;
class UAnimationStateMachineGraph; class UAnimationStateMachineGraph;
@@ -16,6 +18,13 @@ class UAnimStateNode;
class UAnimStateTransitionNode; class UAnimStateTransitionNode;
class UActorComponent; class UActorComponent;
class UWorld; class UWorld;
class UStaticMesh;
class USkeletalMesh;
class UAnimSequence;
class UBlendSpace;
class UTexture;
class UScriptStruct;
class UEnum;
struct FMemberReference; struct FMemberReference;
struct FBPVariableDescription; struct FBPVariableDescription;
@@ -114,6 +123,17 @@ public:
static FString FormatName(const FMemberReference &Ref); static FString FormatName(const FMemberReference &Ref);
static FString FormatName(const FBPVariableDescription &Var); static FString FormatName(const FBPVariableDescription &Var);
static FString FormatName(const UClass *Class); static FString FormatName(const UClass *Class);
static FString FormatName(const UMaterial *Material);
static FString FormatName(const UMaterialInstance *MaterialInstance);
static FString FormatName(const UMaterialFunction *MaterialFunction);
static FString FormatName(const UMaterialExpression *Expression);
static FString FormatName(const UStaticMesh *Mesh);
static FString FormatName(const USkeletalMesh *Mesh);
static FString FormatName(const UAnimSequence *Anim);
static FString FormatName(const UBlendSpace *BlendSpace);
static FString FormatName(const UTexture *Texture);
static FString FormatName(const UScriptStruct *Struct);
static FString FormatName(const UEnum *Enum);
//////////////////////////////////////////////////////// ////////////////////////////////////////////////////////
// //
@@ -135,6 +155,17 @@ public:
static bool Identifies(const FString &Name, const UEdGraphPin *Pin); static bool Identifies(const FString &Name, const UEdGraphPin *Pin);
static bool Identifies(const FString &Name, const FMemberReference &Ref); static bool Identifies(const FString &Name, const FMemberReference &Ref);
static bool Identifies(const FString &Name, const UClass *Class); static bool Identifies(const FString &Name, const UClass *Class);
static bool Identifies(const FString &Name, const UMaterial *Material);
static bool Identifies(const FString &Name, const UMaterialInstance *MaterialInstance);
static bool Identifies(const FString &Name, const UMaterialFunction *MaterialFunction);
static bool Identifies(const FString &Name, const UMaterialExpression *Expression);
static bool Identifies(const FString &Name, const UStaticMesh *Mesh);
static bool Identifies(const FString &Name, const USkeletalMesh *Mesh);
static bool Identifies(const FString &Name, const UAnimSequence *Anim);
static bool Identifies(const FString &Name, const UBlendSpace *BlendSpace);
static bool Identifies(const FString &Name, const UTexture *Texture);
static bool Identifies(const FString &Name, const UScriptStruct *Struct);
static bool Identifies(const FString &Name, const UEnum *Enum);
//////////////////////////////////////////////////////// ////////////////////////////////////////////////////////