Closed englercj closed 9 years ago
Why you decided to have a "next" argument that you have to call in the middlewares? Why not simply return true (to continue) or false (to abort the chain)? I think the "next" thing is prone to errors (you forgot a next() call and you are done).
Maybe what you can pass along is the loader instance, so you can add from inside a middleware more assets if you need, and if you don't, you don't mess anything. Also you have (from the middleware standpoint) more control over the loader if you need it. That will solve your issue and would not need another method, mostly because if later you need another type of loader that parses something like a manifest you would need to add a new method (addWhatever).
Why you decided to have a "next" argument that you have to call in the middlewares?
Because middleware need not be synchronous, For caching middleware that uses IndexedDB the API is async, and you don't want the next to run until you have completed loading the data from the DB. That wouldn't be possible with a true
/false
return system.
Maybe what you can pass along is the loader instance, so you can add from inside a middleware more assets if you need, and if you don't, you don't mess anything.
I do this right now, the middleware function runs in the context of the loader (this
is the loader instance). The problem is if you just do normal adding of stuff to the queue then the progress will "jump back" a certain percentage because now there are more files in the queue. That is the problem I am trying to solve. You can add files right now, but doing so makes progress be wrong.
Ah, that makes perfect sense. Regarding the second issue: Well, yes, but maybe you can solve that by simply loading the manifests first and waiting for them to load before you emit any progress event. After all, if you start loading assets and later decided to add a manifest (while the loader is still loading assets) you will have the jumping-progress issue anyways.
Yeah that was what I proposed in the original post, was to load all manifests then process the queue and emit progress. But in the case that a normal resource loads a manifest...ugh...
I agree... but you don't have any other options if a normal resource loads a manifest. In any case, what I proposed is not to have an "addManifest" method, but simply use the "add" method. Usually you have a big list of assets that you want to load (either normal assets or manifests). So, you defer the loading/parsing until all the assets are registered (with a setTimeout(0) for example) and then you will have enough information on what to do with the list of assets to load (if you have any manifests you can parse them and add items to the load queue, and then start loading).
You can't tell a manifest from not a manifest unless the user tells you. Anything can be a manifest, it just depends on what the parser for it does. There is no way to look at a URL and tell if the parser that runs later after it loads will add more.
I'm not talking about a manifest that I define, "manifest" here means "any object that when loaded will cause a middleware to load more resources". Examples are spine atlas files, tiled tilemaps, phaser asset packs, etc.
Which is another reason it doesn't make sense to split them because a user of this library and middleware they didn't write may not know that will cause extra files to load. This is a very hard problem to solve.
Hard indeed... In that case, I agree with you that you need another method to add manifests. But if a manifest loads more manifests, you will still have the same issue.
I don't think there is a better way and maybe the only possible way to "alleviate" this by letting the user set the total number of files/assets beforehand somehow. In that way, you will never experience a jump in the loading progress (note that number should only be used to calculate the total progress for reporting only. I mean, if the user set that value to 1 and loads a manifest with 10 more files, that number needs to be adjusted and you will have a jump, but hey, at least it's better that the "always jump" option)
Interesting problem, I have a couple thoughts, but I don't know the system well, so forgive me if some of it is way off base:
Always load manifest files first. When a manifest file has loaded, it shouldn't count towards the progress bar.
When a manifest file loads, the assets that get added should cause a dynamic update to the loader. Whatever position it is currently in (say 20%), it uses that as the start, and creates a new 0-100% system between 20% and 100% (if that makes sense, weird for me to explain.) This would prevent the loader from decreasing, and could account for new assets being added dynamically.
A bit roundabout way of explaining it, hopefully it makes sense.
@amadeus Interesting, so instead of adding new files changing the progress it just makes the remaining space "longer". I'm going to play with that a bit...
I'm now using an async queue for managing resources, and each time a new resource is added to the queue the progress chunk that is added to the running progress for each resource is evaluated again.
The end result is as resources are added the bar will appear to "slow down" but it will never go backwards, and will always be the correct progress.
Change was made in: 33cd44790913f454494a63db9de0dc9c928da837
Right now if I have a parser (say of a manifest) that then loads many other files, then progress events for the loader are pretty much borked. Needs to be a way to know how many files will be loaded for proper progress events, otherwise the progress may decrease at points (which is undesired behavior).
Other loaders use a type-based "add" system that allows users to add a resource of a certain type which means we "know" how many resources will eventually need to be loaded.
Maybe have a different type of
.add
specifically for manifests (.addManifest
) that makes that resource not be part of the progress. All manifests are first loaded, parsed (which can add resources to the queue), then the queue is loaded and progress events work as expected.