Bug in AssetListLoader

Help & Support
#1

hey @yaustar, probably one for you.

There’s a bug in the Asset List Loader causing a race condition. In the loader the total is being incremented however the load is also being called in the same block. This means that sometimes an asset can call the onLoad and evaluate the count whilst the total isn’t correct. Ideally, the total should be calculated before any load is called on the assets

#2

Is it this issue? Should AssetListLoader take into account assets that are currently loading · Issue #2967 · playcanvas/engine · GitHub

#3

Hmm, it looks related. Can’t quite tell if it’s identical tho. Sometimes though if I pass a list of 40 assets, I will get a load Complete when 30 are loaded, because it still hasn’t completed incrementing the total value.

I think it can be fixed with this patch

load(done, scope) {
  var i = 0;
  var l = this._assets.length;
  var asset;
  
  // this._total = l;
  this._count = 0;
  this._failed = [];
  this._callback = done;
  this._scope = scope;
  
  this._registry.on("load", this._onLoad, this);
  this._registry.on("error", this._onError, this);

  this._total = this._assets.filter(asset => !asset.loading && !asset.loaded).length

  for (i = 0; i < l; i++) {
    asset = this._assets[i];
  
    if (!asset.loading && !asset.loaded) {
      this._registry.load(asset);
    }
  }
}
1 Like
#4

If there is a way you could create a repro project, that would be great.

The other thing to consider is that this is private API and I don’t think is used in the engine only in tests so it may be moved :thinking:

#5

It’s related to network conditions and cache so it would mean throttling mid way through the load call. I guess a semi relaible test case would be to have a list of smaller assets that can load and return quickly followed by a huge hdr that would take a while to load. You’ll find that the onLoad and ready gets called before the HDR is loaded

#6

Agree with the fix, the total should be calculated before the load is called

I believe the PR fixes it as it does a similar thing to you but using a set instead of total Asset list loader should only complete when all assets in list are loaded by yaustar · Pull Request #2969 · playcanvas/engine · GitHub

As this is only used in tests and is currently private API, I’m considering moving it to the tests folder only

#7

@Mark_Lundin On closer look it is not used anywhere besides it’s own tests. I’m going to double check with the original implementer to double check.

Current consideration is to remove it from the engine to unbloat it a bit but add it as a utility script in the repo somewhere?

What are you thoughts on this?