FAIDataProviderValueDetails::OnBindingChanged resets default value instead of DataField (5.6)

Development Pipeline & Plugins
, ,
1

This came up for us because we have a local system that cribs a lot of how EQS does its UI.

In 5.6 UEnvQueryNode::PostEditChangeProperty started clearing/setting the DataField value when you change the binding.

				// If current field name is set but there are no matching names then reset the binding to the default since it's invalid.
				// This could happen when copy /pasting FAIDataProviderValue based properties of different types.
				else if (MatchingProperties.IsEmpty() && !NameValue.IsNone())
				{
					AIDataProviderValue->DataField = FName();
					AIDataProviderValue->DataBinding = nullptr;
				}



				// If current field name is set but there are no matching names then reset the binding to the default since it's invalid.
				// This could happen when copy /pasting FAIDataProviderValue based properties of different types.
				else if (MatchingProperties.IsEmpty() && !NameValue.IsNone())
				{
					AIDataProviderValue->DataField = FName();
					AIDataProviderValue->DataBinding = nullptr;
				}

Specifically, if you clear the binding, it clears DataField.

FAIDataProviderValueDetails::OnBindingChanged will then validate this after the binding changes, with an identical comment, and will ensure if not set. It then seems to try to set it the same way, but with a difference (that I think is an error)

		// If current field name is set but there are no matching names then reset the binding to the default since it's invalid.
		// This could happen when copy /pasting FAIDataProviderValue based properties of different types.
		else if (MatchingProperties.IsEmpty() && !NameValue.IsNone())
		{
			ensureAlwaysMsgf(false, TEXT("We expect UEnvQueryNode::PostEditChangeProperty to prevent "
				"copying an incompatible provider value on top of an existing one before calling NotifyAssetUpdate and refreshing the details."));
			DataBindingProperty->ResetToDefault();
			DefaultValueProperty->ResetToDefault();
		}

Specifically, it calls DefaultValueProperty->ResetToDefault instead of DataFieldProperty = FName() (or ResetToDefault)

This means that it doesn’t actually fix the bad data, and will keep ensuring it every time you click on it (you can set and then reset the binding yourself to fix it)

I believe it should do the same as the EQS code - clear DataField, and leave DefaultValueProperty alone.

2

Steps to Reproduce
This would require bad data already present in order to reproduce - it happens for us due to game code we have - but likely won’t reproduce for _most_ UE 5.6 users.

3

This was changed to fix a bug that was discovered from our FN team where copy/pasted values would also change the DataField to a wrong value (e.g. copying a float into an int would change DataField=FloatValue. Since the change, we noted that copy/paste was working as expected and have not had other reports.

Is the ensure triggering when resetting the provider value in editor? I know it is happening in some of your custom code, but are there any steps we can attempt to run on our side to see if it happens inside EQS as well?

-James

4

My best guess for a repro would be if someone had bad data in 5.5 and then updated to 5.6 they’d get stuck with a permanent ensure that wouldn’t fix things.

You can simulate this state by skipping the fixup code in EnvQueryNode::PostEditChangeProperty.

Reset a data binding from function->no function and DON’T run the code

				// If current field name is set but there are no matching names then reset the binding to the default since it's invalid.
				// This could happen when copy /pasting FAIDataProviderValue based properties of different types.
				else if (MatchingProperties.IsEmpty() && !NameValue.IsNone())
				{
					AIDataProviderValue->DataField = FName();
					AIDataProviderValue->DataBinding = nullptr;
				}

Then, if you click on that node in the EQS editor, it will ensure every time, and never clean itself up, because it’s resetting DefaultValue and not DataField.

That at least lets you test the ensure, even if it’s not a “natural” way to get into that state.

5

I believe you are correct that this was an error. We have a change in review to swap from DefaultValueProperty->ResetToDefault() to DataFieldProperty->ResetToDefault(). I will keep tabs on the review and let you know if/when it gets submitted or if other questions from the team pop up around the change. I know it was originally done for an FN bug, so I believe the team will want to hear their input and testing before committing it.

-James