2.6 KiB
UMCPHandler_CreateMaterialAsset Refactoring Notes
What was done
-
Switched to plain-text output. The handler now overrides
Handle(Json, FStringBuilderBase&)instead of the JSON variant. Output is concise key-value lines. -
Replaced hand-rolled enum parsing with
MCPUtils::StringToEnum. The long if/else chains for Domain and BlendMode are gone. StringToEnum handles validation and sends a clear error message on bad input. The old code silently ignored invalid values; now it reports them. -
Removed all
UE_LOGcalls. Errors and status go through the output device. -
Trimmed includes. The old file included ~30 headers for material expression types, serialization, GUIDs, etc. that this handler does not use. Kept only what's needed for material creation.
-
Removed the SEH wrapper extern declaration. This handler never called
TryAddMaterialExpressionSEH; that declaration belongs in whatever handler actually uses it. -
Concise output. Removed the
saved: trueecho (only report if save fails). Don't echo input parameters back. -
Parse enums before creating the asset. If Domain or BlendMode are invalid, we now fail before creating the asset, rather than creating it and then silently ignoring the bad value.
What looks good
-
The existence check via
MCPAssets<UMaterial>with.EAny()is a clean use of MCPAssetFinder. -
The
PreEditChange/PostEditChangebracketing andSaveMaterialPackageflow is correct.
Areas for potential further work
-
FormatName not used for the created material. The output uses
GetPathName()for the created material. This is arguably fine for a creation response (the caller needs the exact path), but could be switched toMCPUtils::FormatName(MaterialObj)for consistency with other handlers. I keptGetPathName()because the caller likely needs the full path to reference it later, and I wasn't sure FormatName would produce the same thing. -
The existence check searches by name only, not by path. If a material named "M_Foo" exists at
/Game/OtherFolder/M_Foo, the handler will refuse to create/Game/Materials/M_Foo. This was the pre-existing behavior and I left it alone, but it may be overly conservative. A more precise check would beExact(PackagePath / Name)using the full asset path. -
No batch support. This handler creates a single material. Batching could be useful if the caller wants to create several materials at once, but material creation is less common than node/pin operations, so this may not be worth the complexity.