OnSyncLoadPackage is not being triggered in the correct places in LoadPackageInternal

Development Platform & Builds
, ,
1

The OnSyncLoadPackage delegate is being triggered for cases where we do not synchronously load a package, such as when the package is already loaded or isn’t found. I can see why this might be a good idea, in theory - the delegate is triggered in cases where we’re attempting to load a package synchronously. But in practice, it can become very noisy depending on the load pattern of your game. I wonder if we could move where these delegates are triggered, or add another delegate that is triggered only when a synchronous load will happen.

The first place the delegate is triggered is just after the check for ShouldAlwaysLoadPackageAsync(), just below the comment that begins with “// This delegate is not thread-safe and the subscribers are mostly interested by sync loads”. Would it be better to move the triggering to be inside the check for “if (RequestID != INDEX_NONE)”, so that it’s only triggered if we’re going to call FlushAsyncLoading?

The second place the delegate is triggered is a little further down, just before the comment “// Set up a load context”. There are lots of ways to exit this function without actually loading a package, and I wonder if it would be better to move the triggering down inside the check for “if ((LoadFlags & DoNotLoadExportsFlags) == 0)”, just before the call to LoadAllObjects().

In my testing, this seems to eliminate all of the spurious triggers of the OnSyncLoadPackage delegate (most easily tested by logging a warning when the delegate is triggered). Do you see any problems with these changes? Is this something Epic could consider making on your side?

2

Steps to Reproduce
Mostly N/A - loading happens constantly in every project. But an easy way to see exactly what I mean is:

  1. Create a method (with the OnSyncLoadPackage signature) somewhere that just writes the package name to the log. (best if in a project that is fully set up to async load its assets, but you can see some of the problem in any project)
  2. Subscribe that method to the OnSyncLoadPackage delegate.
  3. Trigger various loading scenarios in the project (load a map, navigate through UI, etc.).
  4. Observe the warnings that are generated.

One thing you should notice, regardless of how your project is set up, is that you are seeing the warning you added for many of the same package, even though they’re already in memory.

3

Hi Ron,

Unfortunately, I don’t think this is something that we can change without a deprecation as this might change the expected behavior of the delegate. In this case, it might not be if a package is actually loading or not but to track and what what part of the code issue such a load instead of opting for the async API for example. Making the change you are suggesting would end up hiding those tracking cases for example. If you are worried about tracking actual load as in your case then you can either use completion callbacks in that case of the OnEndLoadPackage delegate instead of the OnSyncLoadDelegate. Be aware though that in the case of OnEndLoadPackage will be call for both sync load and async load package completion.

Hope that helps!

Thanks!

Francis