ULocalPlayerSaveGame::AsyncSaveGameToSlotForLocalPlayer has an internal bug when multiple async requests are launched before a request completes

When AsyncSaveGameToSlotForLocalPlayer is called multiple times before a save completes, it can trigger the ensure(CurrentSaveRequest > LastSuccessfulSaveRequest) or ensure(CurrentSaveRequest > LastErrorSaveRequest).

This is due to the fact that ULocalPlayerSaveGame::ProcessSaveComplete always operates on the latest CurrentSaveRequest instead of the of the SaveRequest representing the async request. So if multiple async saves are requested before one completes, the first that completes will mark the latest save request as the latest one processed while that request hasn’t completed yet. ISaveGameSystem uses a Task System FPipe so I believe they should always be processed in FIFO order (unless there are prerequisites).

Suggested fix is for ULocalPlayerSaveGame::ProcessSaveComplete to use the SaveRequest passed in which represents the the async request:

	// Now that a save completed, update the success/failure
	// Every time CurrentSaveRequest is incremented it will result in setting either success or error, but there could be multiple in flight that are processed in order
	if (bSuccess)
	{
		//ensure(CurrentSaveRequest > LastSuccessfulSaveRequest);
		//LastSuccessfulSaveRequest = CurrentSaveRequest;
		ensure(SaveRequest > LastSuccessfulSaveRequest);
		LastSuccessfulSaveRequest = SaveRequest;
	}
	else
	{
		//ensure(CurrentSaveRequest > LastErrorSaveRequest);
		//LastErrorSaveRequest = CurrentSaveRequest;
		ensure(SaveRequest > LastErrorSaveRequest);
		LastErrorSaveRequest = SaveRequest;
	}

	HandlePostSave(bSuccess);
}



Steps to Reproduce
Perform 2 ULocalPlayerSaveGame::AsyncSaveGameToSlotForLocalPlayer any complete

First async request: CurrentSaveRequest becomes 1

Second async request: CurrentSaveRequest becomes 2

First async load completes

LastSuccessfulSaveRequest becomes CurrentSaveRequest which is 2

Second async load completes

ensure trips ensure(CurrentSaveRequest > LastSuccessfulSaveRequest); since the current save request was already assigned to LastSaveRequest

Hello Matthew,

Yes, I think your analysis is correct and your suggested fix should work fine. You can safely make that change to your local engine build. I believe this was the originally intended behavior based on the comment and the fact that it does pass the save as a payload. I do think it might make more sense to leave the ensures as checking CurrentSaveRequest, but only update it to SaveRequest.

I filed UE-353812 to cover this issue and will look to fix this in 5.8. I believe the multiple save behavior works correctly other than the ensure (which is usually disabled on shipping builds), and I’ll put enough info in the public bug so others can apply the fix themselves.

-Ben Zeigler