[BUG][STRING TABLES][REDIRECTORS] FTextHistory_StringTableEntry does not obtain the proper TableId if the referenced string table was loaded asynchronoulsy through the redirector

Development Platform & Builds
, ,
1

Hello,

While working on some feature in our project we have faced an issue with some string tables. After an investigation, we have found that there is a bug in the asynchronous path of the string table loading logic. Please find details below.

[Simplified] description of the issue:

1. If there is a string table StringTableA with the package path /Game/<SomePakagePath>/StringTableA, and

2. StringTableA was moved to a new location with a redirection asset creation, so that:

a. /Game/<SomeNewPakagePath>/StringTableA is a new package path for the moved string table, and

b. /Game/<SomePakagePath>/StringTableA now points to the created redirector asset, and

3. FTextHistory_StringTableEntry is created using the old string table path (table id) /Game/<SomePakagePath>/StringTableA, and

4. StringTableA is loaded asyncronously, then

5. After the StringTableA is loaded, the FTextHistory_StringTableEntry::StringTableReferenceData->TableId will not be properly updated to hold the real new table id (which is now /Game/<SomeNewPakagePath>/StringTableA), but will be set instead to the old table id value (/Game/<SomePakagePath>/StringTableA).

6. The incorrect value of FTextHistory_StringTableEntry::StringTableReferenceData->TableId (which is not the real new table id, but instead the old table id (before the string table was moved)) will cause the issue when the affected TextHistory_StringTableEntry is saved as a part of some asset (please do the provided repro steps to see the problem in action).

Some additional notes on the problem:

1. The hidden implicit redirectors application to soft paths during the serialization is a part of the problem.

2. The FTextHistory_StringTableEntry::Serialize(…) has flawed implementation. In particular, the serialized value of TableId can be changed between the different executions during the package saving (harvesting / saving steps):

`void FTextHistory_StringTableEntry::Serialize(FStructuredArchive::FRecord Record)
{
…
if (BaseArchive.IsLoading())
{
…
}
else if (BaseArchive.IsSaving())
{
FName TableId;
FTextKey Key;
if (StringTableReferenceData)
{
StringTableReferenceData->GetTableIdAndKey(TableId, Key);
}

   Record << SA_VALUE(TEXT("TableId"), TableId);          // <== HERE: different executions during the package saving
   Key.SerializeAsString(Record.EnterField(TEXT("Key"))); //           will see different values for the TableId

}

// Collect string table asset references
if (StringTableReferenceData)
{
StringTableReferenceData->CollectStringTableAssetReferences(Record); // <== TableId will be changed here (after the
} // first serialization above)
}`These things contribute to the problem, but for the real cause please see the next post (post characters limit) …

2

Steps to Reproduce

1. Unzip the attached project (strtblrdrbug.zip)

2. Generate Visual Studio project files for the repro project (strtblrdrbug/strtblrdrbug.uproject)

3. Open the generated project solution (strtblrdrbug/strtblrdrbug.sln)

4. Run the project

5. After editor is opened, open the “Output Log” window and search for the “string table redirection bug” string:

[Image Removed]

6. Observe the found “[STRING TABLE REDIRECTION BUG][NO BUG MANIFESTATION] => The test package has been successfully saved” log entry that means the bug was not triggered

7. Close the editor

8. In Visual Studio, open the TestEngineSubsystem.cpp (Solution->Games->strtblrdrbug->Source->strtblrdrbug->Private->TestEngineSubsystem.cpp)

9. Change (line 18) value of the TestMode variable from ETestMode::EnforceBugPreventionMode to ETestMode::EnforceBugManifestationMode:

[Image Removed]

10. Compile and run the project again without debugging

11. Observe the project crash:

[Image Removed]

12. Close the crash report window

13. Run project again, now with debugging and catch the issue in action in the debugger:

[Image Removed]

3

Continuation of the original topic post …

Real cause of the issue:

The real cause of the issue is that asynchronous string table loading logic does not report the proper real table id of the loaded string table if the table was loaded by the package path that points to the redirector instead of the real table asset.

The synchronous path does not have this issue and reports the proper table id even if table was loaded through the redirector. The lack of the issue with the synchronous loading logic leads us to the solution. Please see below.

Our solution to the issue:

We have fixed the issue in our project by applying the same redirectors following logic that is applied inside of the synchronous loading codepath:

`// ============================================================
// StringTable.cpp
// ============================================================
void HandleStringTableAssetAsyncLoadCompleted(…)
{
…
// Calculate the name of the loaded string table based on the package name
FName LoadedStringTableId;
if (InLoadingResult == EAsyncLoadingResult::Succeeded)
{
// BEGIN ~ !!! NEW LOGIC TO FIX THE ISSUE !!! ~ BEGIN
// If string table was moved and redirector asset was created, the InLoadPackage
// will be a package for the created redirector asset.
// AsyncLoadingState.RequestedTableId contains the original table ID
// and basically is a name of the string table asset object. If redirector
// asset is involved, the AsyncLoadingState.RequestedTableId actually points
// to the redirector object inside of redirector asset.
UObject* FoundObject = FindObject(nullptr, *AsyncLoadingState.RequestedTableId.ToString());

// If found object is redirector object, we have to follow the redirection:
while (UObjectRedirector* Redirector = Cast(FoundObject))
{
FoundObject = Redirector->DestinationObject;
}

// If we found no object, we have to report “None” (initial value of LoadedStringTableId)
// as a Table ID of loaded string table => this is exactly what sycn path do in a case of failed load.
if (FoundObject)
{
// Now we need to update the InLoadedPackage and set it to the package of real
// string table asset, not the redirector asset.
InLoadedPackage = FoundObject->GetPackage();
// END ~ !!! NEW LOGIC TO FIX THE ISSUE !!! ~ END
check(InLoadedPackage);
const FString LoadedPackageNameStr = InLoadedPackage->GetName();
LoadedStringTableId = *FString::Printf(TEXT(“%s.%s”), *LoadedPackageNameStr, *FPackageName::GetLongPackageAssetName(LoadedPackageNameStr));
} // One more new line to close the added “if (FoundObject)” :slight_smile:
}

// Notify any listeners of the result
for (const FLoadStringTableAssetCallback& LoadedCallback : AsyncLoadingState.LoadedCallbacks)
{
check(LoadedCallback);
LoadedCallback(AsyncLoadingState.RequestedTableId, LoadedStringTableId);
}
}`We are interested to know if this issue is known, and if it is known, is it planned to be fixed ? Also, can we consider our solution as a proper solution to the issue ? Are there any drawbacks that we have overlooked?

4

Ping ?