After investigating a problem one of our designers were having around their blueprints resetting some variable values on compilation, I’ve noticed they had variables with the same name created under some functions. Because these blueprints can have a lot of variables and functions, it seems to be really easy to make the mistake of creating a local variable with the same name and there doesn’t seem to be any warning mechanism when you do so. Similar thing is generally not allowed with functions and other stuff and the object is automatically renamed to have a suffix which doesn’t seem to be the case with blueprint variables & local function variables. I’ve implemented a local fix by doing a `FindUniqueKismetName` on variable operations but first wanted to ask about if there could be any test scenarios I could be missing and need to test around to ensure this doesn’t cause a problem and secondarily ask if Epic would be interested in changing the behaviour here as this feels like a possibly disruptive behaviuor that can easily go unnoticed and cause problems but this seems to have been a behaviour allowed for a long time and maybe there is a reason behind it?
BP_BadVariable.zip(4.84 KB)
Hello again! I’ve looked into this a bit and, indeed, easy to repro and run into.
After some digging I found that the culprit is SBlueprintPaletteItem::OnNameTextVerifyChanged() which after a refactor long ago, fails to resolve the outer BlueprintObj for local variables. That causes name validation to be skipped both when editing and committing the name change, at least from the palette widget. Local name collisions are caught later, but name collisions with member names are not caught during text committing or later. Here is my patch for this issue in engine code. It’s currently in review but you may choose to adopt it already.
In SBlueprintPaletteItem::OnNameTextVerifyChanged():
else if (ActionPtr.Pin()->GetTypeId() == FEdGraphSchemaAction_K2LocalVar::StaticGetTypeId())
{
FEdGraphSchemaAction_K2LocalVar* LocalVarAction = (FEdGraphSchemaAction_K2LocalVar*)ActionPtr.Pin().Get();
OriginalName = (LocalVarAction->GetVariableName());
// BEGIN ENGINE MOD
// Expected outer: UFunction, use for checking local variable name collision
ValidationScope = CastChecked<UStruct>(LocalVarAction->GetVariableScope());
// Expected outer of UFunction in this context: UBlueprintGeneratedClass, resolve its Schema for checking name collision with member variables
UClass* VarClass = ValidationScope->GetTypedOuter<UClass>();
// END ENGINE MOD
if (VarClass)
{
UBlueprint* BlueprintObj = UBlueprint::GetBlueprintFromClass(VarClass);
TArray<UEdGraph*> Graphs;
BlueprintObj->GetAllGraphs(Graphs);
if (Graphs.Num() > 0)
{
Schema = Graphs[0]->GetSchema();
}
}
}
No problem! In the end I went for a slightly different fix, which is a bit more future proof. It’s merged into //UE5/Main now at CL 53666857. Either fix should work.
The new fix, tldr:
/** Reference to a local variable (only used in 'docked' palette) */
USTRUCT()
struct FEdGraphSchemaAction_K2LocalVar : public FEdGraphSchemaAction_BlueprintVariableBase
{
GENERATED_USTRUCT_BODY()
public:
// Simple type info
static FName StaticGetTypeId() {static FName Type("FEdGraphSchemaAction_K2LocalVar"); return Type;}
virtual FName GetTypeId() const override { return StaticGetTypeId(); }
UE_API virtual int32 GetReorderIndexInContainer() const override;
FEdGraphSchemaAction_K2LocalVar()
: FEdGraphSchemaAction_BlueprintVariableBase()
{}
FEdGraphSchemaAction_K2LocalVar(FText InNodeCategory, FText InMenuDesc, FText InToolTip, const int32 InGrouping, const int32 InSectionID)
: FEdGraphSchemaAction_BlueprintVariableBase(MoveTemp(InNodeCategory), MoveTemp(InMenuDesc), MoveTemp(InToolTip), InGrouping, InSectionID)
{}
virtual bool IsA(const FName& InType) const override
{
return InType == GetTypeId() || InType == FEdGraphSchemaAction_BlueprintVariableBase::StaticGetTypeId();
}
// BEGIN CHANGE, also made GetVariableClass virtual in parent
virtual UClass* GetVariableClass() const override
{
UObject* VariableScope = GetVariableScope();
return VariableScope ? VariableScope->GetTypedOuter<UClass>() : nullptr;
}
// END CHANGE
};
Regarding this:
“this seems to have been a behaviour allowed for a long time and maybe there is a reason behind it?”
I found an old internal jira ticket to address this, which leads me to believe there isn’t a reason to allow local and member variables to have the same name. I believe CL 18281573 introduced this as a regression.
Thanks a lot for the proper fix Zhi, fix I had was working after the commit, this now shows proper error message while typing as well, I will be submitting it into our codebase asap. And yeah regression from 2021 makes sense, I went back few editor versions to test it to make sure it was the behaviour there as well but didn’t go back to any 4.x versions.