Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asynchronous coroutines-based asset manager: AssetStorage #258

Merged
merged 17 commits into from
Apr 10, 2020

Conversation

czyzby
Copy link
Member

@czyzby czyzby commented Mar 29, 2020

See #182.

@czyzby czyzby added assets Issues of the ktx-assets module async Issues of the ktx-async module labels Mar 29, 2020
@czyzby czyzby added this to the 1.9.10 milestone Mar 29, 2020
@czyzby czyzby self-assigned this Mar 29, 2020
@czyzby
Copy link
Member Author

czyzby commented Mar 29, 2020

GitHub seems to lag behind Travis, the CI builds have passed so far.

@czyzby czyzby mentioned this pull request Mar 30, 2020
6 tasks
@Quillraven
Copy link
Contributor

Quillraven commented Mar 31, 2020

I tried a very quick&dirt replacement from assetmanager to assetstorage in my LoadingScreen and seems like the loading time went down from an average ~2.70 seconds to an average ~1.50 seconds which is great!

However, I am not sure if the test was 100% perfect so I tried to replace now all assetmanager stuff with assetstorage but I am struggling to achieve something. Since I am no expert with coroutines I need some help please.

Imagine that I have a method that takes an asset. In a completely different method I am queueing the loading of the asset. How can I get that asset? Seems like await can only be called within a coroutine context? Does that mean that everytime I want to do something with assets I have to wrap it around KtxAsync.launch{} ?

Here is an example:

override fun create() {
        Gdx.app.logLevel = logLevel

        currentFrameBuffer = FrameBuffer(Pixmap.Format.RGB888, VIRTUAL_W, VIRTUAL_H, false)
        nextFrameBuffer = FrameBuffer(Pixmap.Format.RGB888, VIRTUAL_W, VIRTUAL_H, false)

        // init Box2D - the next call avoids some issues with older devices where the box2d libraries were not loaded correctly
        Box2D.init()

        // init asset storage
        KtxAsync.initiate()
        val assets = AssetStorage().apply {
            // we use tmx tiled maps created via the Tiled tool and therefore
            // we use the TmxMapLoader for our assetmanager to be able to
            // load/unload .tmx files
            setLoader(TiledMap::class.java) { TmxMapLoader(fileResolver) }
        }

        // load skin stuff
        KtxAsync.launch {
            val atlas = assets.load<TextureAtlas>(TextureAtlasAssets.UI.filePath)
            val bundle = assets.load<I18NBundle>(I18nAssets.DEFAULT.filePath)

- Note how everything below is now wrapped within the coroutine. Is this the only way?
            // setup context and register stuff that should also be disposed at the end of the game lifecycle
            ctx.register {
                bindSingleton(ShaderPrograms())
                bindSingleton(SpriteBatch(2048))
                bindSingleton(assets)
                bindSingleton(Stage(FitViewport(VIRTUAL_W.toFloat(), VIRTUAL_H.toFloat()), ctx.inject<SpriteBatch>()))
- Here I need to pass in the atlas from above
                bindSingleton(createSkin(atlas))
                bindSingleton(RayHandler(world))
                bindSingleton(Box2DDebugRenderer())
                bindSingleton(OrthogonalTiledMapRenderer(null, UNIT_SCALE, ctx.inject<SpriteBatch>()))
            }

            // we need a multiplexer to react on the following input events
            // UI widget --> Stage
            // keyboard --> InputProcessor (GameEventManager)
            Gdx.input.inputProcessor = InputMultiplexer(gameEventManager, ctx.inject<Stage>())

            // box2d light should not create shadows for dynamic game objects
            Light.setGlobalContactFilter(FILTER_CATEGORY_LIGHT, 0, FILTER_MASK_LIGHTS)

            // initial screen is the loading screen which is loading all assets for the game
            addScreen(
                LoadingScreen(
- this was originally referring to KtxGame but since it is now within a coroutine this was referring to the coroutine context
                    this@Main, // game instance to switch screens
- here I need to pass in the bundle from above
                    bundle,
                    ctx.inject(), // stage
                    ctx.inject(), // assets
                    gameEventManager, // game event manager
                    audioService,
                    world, // physic world
                    ecsEngine, // entity component engine
                    ctx.inject(), // ray handler
                    ctx.inject(), // shaders
                    ctx.inject(), // sprite batch
                    ctx.inject(), // tiled map renderer
                    ctx.inject() // box2d debug renderer
                )
            )
            setScreen<LoadingScreen>()
        }
    }

Another thing that I am missing at the moment is how do I get the progress of the loading? E.g. in my render method I update a progress bar currently with the progress of an AssetManager. How can I achieve that with AssetStorage?

loadingBar.scaleTo(assets.progress)

Anyway, getting late in my zone and I am very tired already... Maybe tomorrow it becomes clearer to me ;)

@czyzby
Copy link
Member Author

czyzby commented Mar 31, 2020

@Quillraven

seems like the loading time went down from an average ~2.70 seconds to an average ~1.50 seconds which is great!

That's great, thanks for testing! Did you use a single thread for loading? You can try using e.g. AssetStorage(asyncContext = newAsyncContext(threads = 4)) to test concurrent loading.

Does that mean that everytime I want to do something with assets I have to wrap it around KtxAsync.launch{}?

You typically obtain the assets via coroutines, yes, but once the assets are loaded/extracted from storage, you have direct references to them and can assign or pass them wherever you need.

For example, at first you can create all of the components that do not require loaded assets in your context. Stuff like stages, batches etc. can all be created before Ktx.launch is called.

When you're done with the asset loading in the coroutine, create and add the other components to the context. This is when you can create your skin that requires an atlas or a screen that uses the loaded assets.

If you want to continue using AssetStorage as a sort of bundle of assets instead of passing individual loaded instances, you can use runBlocking to extract the assets immediately. Keep in mind that this can cause a deadlock if the asset is unloaded, so use with caution.

// Does not need a coroutine:
val texture = runBlocking { assetStorage.get<Texture>("image.png").await() }

You can also go all in, integrate coroutines into more places in your application and make suspend methods in your screens that extract assets from the storage and don't need explicit KtxAsync.launch. Unfortunately, I don't have examples of such applications, so you'd have to read more about Kotlin coroutines and play around with the API.

@czyzby
Copy link
Member Author

czyzby commented Mar 31, 2020

@Quillraven
I suppose when you're used to assetManager.get and not really using coroutines throughout the application, the whole runBlocking syntax is pretty verbose if all you need is a map<path, asset> in the end. I might refactor existing get to getAsync (as encouraged by Kotlin coroutines standards for methods returning Deferred) and change get to a briefly blocking function.

Try using this and let me know if it works for you:

/**
 * Returns an instance of the asset of type [T] loaded from [path]
 * or throws [MissingAssetException] if unloaded.
 */
inline fun <reified T> AssetStorage.getNow(path: String): T = runBlocking { 
  val asset = get<T>(path)
  if (asset.isCompleted) {
    asset.await()
  } else {
    throw MissingAssetException(getIdentifier<T>(path))
  }
}

For future readers: getNow from the example above behaved like the current get. Initially, get returned Deferred like current getAsync does.

@Quillraven
Copy link
Contributor

Quillraven commented Mar 31, 2020

Wow you are fast with responding :D Thanks!

I will try it out some more tomorrow or on the weekend!

Three more things that I found:

  1. the loading bar question from above (edited it in the comment at the end)
  2. I have a MapManager class which has a cache of maps which internally have a reference to their related TiledMap. The setMap function looks like below
  3. My test was definitely correct and the loading is ~1 second faster! I just use the normal AssetStorage() constructor at the moment. Will try multi-threading later :)
fun setMap(mapType: MapType, targetPortal: Int = -1, targetOffsetX: Int = -1) {
        val currentMap = mapCache[currentMapType]
        if (currentMap != null) {
            storeMapEntities(currentMapType)
            LOG.debug { "Storing map entities of map $currentMapType: ${mapEntityCache[currentMapType]}" }

            gameEventManager.dispatchBeforeMapChangeEvent()
            // remove all non-player entities of the current loaded map
            ecsEngine.entities.forEach { entity ->
                if (entity.typeCmp.type != EntityType.PLAYER) {
                    // non player entity -> remove it
                    entity.add(ecsEngine.createComponent(RemoveComponent::class.java))
                }
            }
        }

        // check if new map is already existing. Otherwise, create it
        currentMapType = mapType
- how can the computeIfAbsent LAMBDA return a Map instance if it has to be wrapped around KtxAsync? 
- Or can I use the runBlocking part that you mentioned above? What would be a good approach here?
        mapCache.computeIfAbsent(mapType) { Map(mapType, assets[mapType.asset]) }.apply {
            if (targetPortal == -1) {
                // target portal is not specified -> move to default player start location
                movePlayerToStartLocation(this)
            } else {
                // move player to target portal position
                movePlayerToPortal(this, targetPortal, targetOffsetX)
            }
            createSceneryEntities(this)
            createEnemyEntities(this)
            createSavepoints(this)
            createNPCs(this)
            createItemEntities(this)
            createPortalEntities(this)
            createTriggers(this)
            updateAmbientLight(this)
            gameEventManager.dispatchMapChangeEvent(this)
        }
    }

edit: In the end my current design might not be the best for AssetStorage :D I will keep that in my mind for the next project and set it up differently from the beginning. I will still try to change my current project to it but I am not 100% sure if I am able to do so. Let's see! ;)

@czyzby
Copy link
Member Author

czyzby commented Mar 31, 2020

the loading bar question from above (edited it in the comment at the end)

AssetStorage currently does not support progress tracking. See #259.

There's also a reason why I didn't include it out-of-the-box just yet: AssetManager works fundamentally different and schedules all assets up front. With AssetStorage, you usually load the assets synchronously within a coroutine, so they are not all scheduled up front. This is why AssetManager could easily track total progress as a percent, but in case of AssetStorage it would just go from 0.0 to 1.0 for the first asset, then suddenly from 0.5 to 1.0 as second gets loaded, then 0.66 to 1.0, etc., as more assets get scheduled.

Do you think simple numerical values would be helpful (i.e. number of loaded, scheduled and failed assets)?

Will try multi-threading later :)

Other than giving the storage multiple threads, you have to actually load the assets asynchronously. See the usage examples in the README. Basically comes down to:

// Synchronous within coroutine - one asset loaded after another:
val t1 = storage.load<Texture>("1.png")
val t2 = storage.load<Texture>("2.png")

// Asynchronous - truly parallel loading:
val t1 = async { storage.load<Texture>("1.png") }
val t2 = async { storage.load<Texture>("2.png") }
// Obtaining the assets - will suspend the coroutine before both are loaded:
t1.await(); t2.await()
  • how can the computeIfAbsent LAMBDA return a Map instance if it has to be wrapper around KtxAsync? Or can I use the runBlocking part that you mentioned above? What would be a good approach here?

Am I understanding this correctly that you need either an asset from the storage or null? Try rewriting this with the getNow extension method that I've written in the previous comment or this one:

/**
 * Returns an instance of the asset of type [T] loaded from [path]
 * or returns `null` if asset is absent or unloaded.
 */
inline fun <reified T> AssetStorage.getOrNull(path: String): T? = runBlocking {
  val asset = get<T>(path)
  if (asset.isCompleted) asset.await() else null
}

@czyzby
Copy link
Member Author

czyzby commented Mar 31, 2020

@Quillraven
After your comments I realized that in order for AssetStorage to not get in your way after the assets are loaded, it needs the following methods:

  • get: T - returns asset of T type immediately if it is loaded or throws an exception otherwise. Typically used when you're sure that the loading of assets has finished.
  • getOrNull: T? - returns asset of T type immediately if it is loaded or returns null. A bit safer you use if you're unsure if the assets have loaded yet and have measures to deal with it.
  • getAsync: Deferred<T> - current get implementation, returns a reference to the result. Allows the users to await for the asset within a coroutine, suspending it until the assets are loaded.

Current get API based solely on Deferred is more tedious than LibGDX AssetManager, which defeats the whole purpose of KTX.

@czyzby
Copy link
Member Author

czyzby commented Mar 31, 2020

@Quillraven Thank you for your valuable feedback, I implemented the new accessors that I've mentioned in the previous comment. The documentation was adjusted and the snapshot release should be available soon.

See the latest comment if you're interested in the exact changes (README might be the most interesting, as it has the updated usage examples).

I'd appreciate it if you tested the updated API. Thanks!

@Quillraven
Copy link
Contributor

Quillraven commented Apr 1, 2020

@czyzby Sorry but I don't have time until the weekend. Writing here from my phone ;)

I read a little bit about coroutines and I understand now why you said it is problematic to have a progress.

However, in a loading screen (at least for me) you will most likely load a bunch of assets in the same coroutine context, right?

The user can count this amount himself but somehow he needs to know when the loading of an asset is done. That way you can have a "totalAssetsToLoad" and "assetsLoaded" counter.

Is there a way to get notified when an asset is loaded? Or maybe query in render function all queued assets of the loadingscreen to find out how many are already done? What do you think?

Also, you misunderstood the computeIfAbsent part but no worries, I explained it poorly haha.
The idea is that if a key is missing in the cache then create the entry for it. The TiledMap asset is already loaded there and can be passed to the Map class.
But I think this problem is already solved with the things you mentioned above.

However I still want to think about some code design changes in my project to better use coroutines because from a first glympse they seem to be fairly straightforward and the code can be easier structured than compared to Java Thread concurrency.

The only thing that I would really like to still have is my progress bar haha. I have that in all of my games ;)
You are a smart guy! You can for sure come up with a solution.

@czyzby
Copy link
Member Author

czyzby commented Apr 1, 2020

@Quillraven

The user can count this amount himself but somehow he needs to know when the loading of an asset is done. That way you can have a "totalAssetsToLoad" and "assetsLoaded" counter.

I don't think this is the way to go, as A) you'd have to remember to update the count, B) assets have dependencies that the user might not know about, so we'd end up with e.g. 10 total loaded assets vs 5 supposedly scheduled. It has to be built-in and automatic.

You could however schedule all your assets for asynchronous loading similarly to how AssetManager works. I'm adding loadAsync to make it easier, up to the point of eliminating manual coroutines firing. See loadAsync that I've added in latest commit and the AssetStorage as a drop-in replacement for AssetManager README section.

You're right about progress tracking being an important feature, I might work on this before merging this pull request. I'll let you know when I come up with something.

@czyzby
Copy link
Member Author

czyzby commented Apr 1, 2020

@Quillraven The best I can do is eventually consistent progress tracking. Assets and their dependencies are resolved asynchronously, so we can't reliably track progress without introducing blocking code or breaking things as soon as you use concurrent loading/unloading.

In other words: you will be able to get a progress value in range of [0, 1], but it might not represent the state of all of your assets, since some of them are not even scheduled for loading yet. It will eventually catch up, but not immediately. You might notice some jumps at the very beginning of loading due to scheduled assets being added to progress tracking.

Nothing that initial screen fade in and slower animation speed on progress bar wouldn't fix. ;')

@czyzby czyzby linked an issue Apr 2, 2020 that may be closed by this pull request
5 tasks
@czyzby
Copy link
Member Author

czyzby commented Apr 2, 2020

@Quillraven I included progress tracking with some usage examples in the latest commit. I'd appreciate it if you tested the API again. You probably want to base your loading code on the WithAssetStorage example with a list of deferreds. Thanks.

@Quillraven
Copy link
Contributor

@czyzby Thanks for the updates! Unfortunately, I am too stupid at the moment to understand how to fix my code to make it concurrent :D

Maybe some things are simply not possible the way I want them to be. I was able to rewrite LoadingScreen with your usage example like this:

override fun show() {
        // queue all assets that should be loaded
        MusicAssets.values().forEach { assets.loadAsync<Music>(it.filePath) }
        SoundAssets.values().forEach { if (it != SoundAssets.UNKNOWN) assets.loadAsync<Sound>(it.filePath) }
        TextureAtlasAssets.values()
            .forEach { if (it != TextureAtlasAssets.UI) assets.loadAsync<TextureAtlas>(it.filePath) }
        MapAssets.values().forEach { assets.loadAsync<TiledMap>(it.filePath) }
        ParticleAssets.values().forEach {
            assets.loadAsync<ParticleEffect>(
                path = it.filePath,
                parameters = ParticleEffectLoader.ParticleEffectParameter().apply {
                    atlasFile = TextureAtlasAssets.GAME_OBJECTS.filePath
                }
            )
        }

        stage.clear()
        stage.addActor(loadingBar)
        stage.addActor(touchToBeginInfo)
        loadingBar.centerPosition(stage.width * 0.5f, stage.height * 0.15f)
        with(touchToBeginInfo) {
            color.a = 0f
            centerPosition()
        }
    }

override fun render(delta: Float) {
        loadingBar.scaleTo(assets.progress.percent)

        if (assets.progress.isFinished && !loaded) {
            loaded = true

            touchToBeginInfo += forever(sequence(fadeIn(1f), fadeOut(1f)))
            // ...
}

The problem is, I cannot come to that part because I fail to understand how I can load my skin for scene2d before that. Of course I could rewrite everything, then it would work but that takes more time that I currently don't have, sorry.

To explain it to you:
In my Application create method I am registering the skin to the context:

override fun create() {
        Gdx.app.logLevel = logLevel

        currentFrameBuffer = FrameBuffer(Pixmap.Format.RGB888, VIRTUAL_W, VIRTUAL_H, false)
        nextFrameBuffer = FrameBuffer(Pixmap.Format.RGB888, VIRTUAL_W, VIRTUAL_H, false)

        // init Box2D - the next call avoids some issues with older devices where the box2d libraries were not loaded correctly
        Box2D.init()

        // setup context and register stuff that should also be disposed at the end of the game lifecycle
        ctx.register {
            bindSingleton(ShaderPrograms())
            bindSingleton(SpriteBatch(2048))
            // initialize Kotlin coroutines context
            KtxAsync.initiate()
            bindSingleton(AssetStorage().apply {
                // we use tmx tiled maps created via the Tiled tool and therefore
                // we use the TmxMapLoader to be able to load/unload .tmx files
                setLoader<TiledMap> { TmxMapLoader(this.fileResolver) }
            })
            bindSingleton(Stage(FitViewport(VIRTUAL_W.toFloat(), VIRTUAL_H.toFloat()), ctx.inject<SpriteBatch>()))
- here I create and load the skin
            bindSingleton(createSkin(ctx.inject()))
            bindSingleton(RayHandler(world))
            bindSingleton(Box2DDebugRenderer())
            bindSingleton(OrthogonalTiledMapRenderer(null, UNIT_SCALE, ctx.inject<SpriteBatch>()))
        }

        // we need a multiplexer to react on the following input events
        // UI widget --> Stage
        // keyboard --> InputProcessor (GameEventManager)
        Gdx.input.inputProcessor = InputMultiplexer(gameEventManager, ctx.inject<Stage>())

        // box2d light should not create shadows for dynamic game objects
        Light.setGlobalContactFilter(FILTER_CATEGORY_LIGHT, 0, FILTER_MASK_LIGHTS)

        // initial screen is the loading screen which is loading all assets for the game
        addScreen(
            LoadingScreen(
                this, // game instance to switch screens
                ctx.inject<AssetStorage>()[I18nAssets.DEFAULT.filePath],
                ctx.inject(), // stage
                ctx.inject(), // assets
                gameEventManager, // game event manager
                audioService,
                world, // physic world
                ecsEngine, // entity component engine
                ctx.inject(), // ray handler
                ctx.inject(), // shaders
                ctx.inject(), // sprite batch
                ctx.inject(), // tiled map renderer
                ctx.inject() // box2d debug renderer
            )
        )
        setScreen<LoadingScreen>()
    }

createSkin looks like this:

fun createSkin(assetStorage: AssetStorage): Skin {
    // load textures for skin
    val assets = listOf(
        assetStorage.loadAsync<TextureAtlas>(TextureAtlasAssets.UI.filePath),
        assetStorage.loadAsync<I18NBundle>(I18nAssets.DEFAULT.filePath)
    )

    KtxAsync.launch {
        assets.joinAll()
        Scene2DSkin.defaultSkin = skin(assetStorage[TextureAtlasAssets.UI.filePath]) { skin ->
            // fonts
            add(FontType.DEFAULT.skinKey, getBitmapFont("font24", atlas))
            add(FontType.LARGE.skinKey, getBitmapFont("font32", atlas))

            // default label style
            label { font = skin.getFont(FontType.DEFAULT.skinKey) }
            label(LabelStyles.LARGE.name) { font = skin.getFont(FontType.LARGE.skinKey) }
            label(LabelStyles.MAP_INFO.name) {
                font = skin.getFont(FontType.LARGE.skinKey)
                background = skin[Images.DIALOG_TITLE]
            }

            // setting up other skin stuff ...
        }
    }

    return Scene2DSkin.defaultSkin
}

In my opinion, I now face the problem that return is called before the skin is loaded. I tried to wrap it around "runBlocking" but as you mentioned in the Documentation this ends up in a deadlock.

Also tried your getNow function but get errors there as well:
image

Therefore, sorry but I am most likely the wrong one to ask for good tests because I am not very familiar with concurrency and coroutines. I understand the basics and why errors are happening, but I am not experienced enough to know how to fix them properly.

@czyzby
Copy link
Member Author

czyzby commented Apr 4, 2020

@Quillraven

You don't need getNow, I've included alternatives directly in the library:

  • get is now the equivalent of the getNow that I posted. It either extracts an asset or throws an exception.
  • getAsync behaves similarly to the old get. It returns a Deferred reference to the asset that you can await for.
  • getOrNull is similar to the new get, but returns a null if the asset is not loaded.

The problem is, I cannot come to that part because I fail to understand how I can load my skin for scene2d before that.

Currently you have 2 options:

  • Load the Skin synchronously without the manager, register it and its resources (namely, the texture atlas) in the AssetStorage via add.
  • Add a dummy screen that renders nothing and waits for the crucial assets to be loaded (UI, anything you need on the loading screen). Once the assets are loaded, switch the screen.

I think this is a very good observation as well, we might need something like loadSync, loadBlocking or loadImmediately to load assets completely synchronously for the loading screen and whatnot.

@Quillraven
Copy link
Contributor

Quillraven commented Apr 4, 2020

@czyzby Thank you again for your patience and help! During a long sunny walk outside I had some time to think and I came up with the following: Quillraven/Quilly-s-Adventure#40

It seems to work now and the loading time went down even further to an average of ~0.9 seconds. It did not make a difference if I used 4 threads or 1 thread. I guess the work is too small and the overhead of multi-threading makes it then the same speed as with 1 thread.

Loading bar seems to work the way I am doing it. I scale the texture with interpolation using a delay of 0.1 seconds to reach the new value. It does not jump around and seems to behave the same way it did before.

I will try it out on my phone later, if there are any issues but for now it looks good! Thanks again czyzby.

edit: also works on my phone (Samsung S8) and the loading is incredibly fast there now as well. Before it was a couple of seconds and now <1 second. Great job!

@czyzby
Copy link
Member Author

czyzby commented Apr 4, 2020

@Quillraven
Hey, no problem! I'm really glad that you took the time to test it, I'd honestly ship a version that's much less usable without your feedback.

I guess the work is too small and the overhead of multi-threading makes it then the same speed as with 1 thread.

It's also possible that the asynchronous loading part (i.e. loading the files from drive) isn't as costly as the synchronous part (OpenGL/OpenAL stuff), which has to be done on the same thread. Maybe the performance benefits come from the non-blocking approach and a cleaner implementation, and not entirely the asset loading part.

Loading bar seems to work the way I am doing it. I scale the texture with interpolation using a delay of 0.1 seconds to reach the new value. It does not jump around and seems to behave the same way it did before.

Great, good to know. I reviewed your code - it's looking good, I like how you solved the initialization part. Personally I'd only look into replacing isFinished with coroutines - it's fine as it is, but maybe I'll submit a PR. ;)

Just so you know, I'll work on the synchronous loading tomorrow. If that's OK, I'd like to ask you to test the changes or at least review the API. This will solve your initial problem of having to load skin assets before switching to the loading screen (not that you did it wrong with coroutines).

Can I ask if you'll be switching from AssetManager to AssetStorage once I release a stable KTX version?

@Quillraven
Copy link
Contributor

Can I ask if you'll be switching from AssetManager to AssetStorage once I release a stable KTX version?

That's the plan ;)

If that's OK, I'd like to ask you to test the changes or at least review the API. This will solve your initial problem of having to load skin assets before switching to the loading screen (not that you did it wrong with coroutines).

Sure! I will wait with the merge of the AssetStorage PR then I can easily try it out in the current master again and let you know.

@czyzby
Copy link
Member Author

czyzby commented Apr 5, 2020

I added loadSync loading variants that block the thread until an asset is loaded to solve the problem of loading initial resources, e.g. UI for the loading screen. Note that this can be achieved without loadSync, but I felt that it would be easier for beginners that are not familiar with coroutines.

I believe is the last feature needed to make AssetStorage on par with AssetManager. Once this gets approved, I'll merge it to develop, setup another beta release and implement further features in #259.


@Quillraven Your application setup with coroutines is fine as it is, but could you also test loadSync? E.g. to load the texture atlas for the skin. Much appreciated.

@czyzby czyzby mentioned this pull request Apr 5, 2020
3 tasks
@Quillraven
Copy link
Contributor

will try it out and give feedback on Friday.

sorry work life is very busy at the moment. that is why I don't have time during the week.

@czyzby
Copy link
Member Author

czyzby commented Apr 8, 2020

@Quillraven No problem, I understand. Take your time. I also planned on doing a final review of my code on weekend and merging it into develop.

@Quillraven
Copy link
Contributor

Quillraven commented Apr 10, 2020

Had some time now to try it out and if used correctly, it works (at least for me) :)

However, I also tried to break it and I did not fully understand the behavior. Let's assume following calls:

KtxAsync.launch {
            // load textures for skin
            val atlas = assetStorage.loadSync<TextureAtlas>(TextureAtlasAssets.UI.filePath)
            val uiAtlas = assetStorage.loadAsync(TextureAtlasAssets.UI)
            val bundle = assetStorage.loadAsync(I18nAssets.DEFAULT)

Note: I have an overload of loadAsync with my enums but I did not do that for loadSync yet.
Anyway, calling it this way works. The referenceCount for the UI TextureAtlas is initially 1 (loadSync) and becomes 2 once loadAsync is done. From my understanding this is correct.

Now let's change the call order:

KtxAsync.launch {
            // load textures for skin
            val uiAtlas = assetStorage.loadAsync(TextureAtlasAssets.UI)
            val bundle = assetStorage.loadAsync(I18nAssets.DEFAULT)
            val atlas = assetStorage.loadSync<TextureAtlas>(TextureAtlasAssets.UI.filePath)

Note how loadSync is now called after the loadAsync method. Here we get following exception

Exception in thread "main" ktx.assets.async.MissingAssetException: Asset: Identifier(path=ui/UI.atlas, type=class com.badlogic.gdx.graphics.g2d.TextureAtlas) is not loaded.

Of course this is not a real live example because why would you load the same asset synchronously and asynchronously at the same time. And we programmers never make mistakes ;)

But I found it interesting that it behaves differently. It is actually mentioned in your documentation for loadSync. That is why I assume it is correct but I don't understand why it does not deliver the same result as the first example.

Can you explain it to me please?

edit: one more thing. I also tried it out correctly by calling it only once and before the KtxAsync.launch block. I think this is the real use case for this method and it worked!

@czyzby
Copy link
Member Author

czyzby commented Apr 10, 2020

@Quillraven
You shouldn't call loadSync in a coroutine - it defeats the purpose, as it blocks the coroutine to wait for the asset. Use only load and loadAsync within coroutines.

In your example, when you call loadAsync twice, you're scheduling loading of 2 assets and then immediately proceed to load an asset without its dependencies with loadSync. That's why you're getting the exception - loadSync will not wait for the assets to be loaded asynchronously, and instead throws an exception, since if it blocked the current thread to wait for the dependencies, it would risk deadlocking the main thread.

You should use loadSync only outside of coroutines and ideally before other assets are scheduled for loading with load/loadAsync (unless they are already loaded, of course).

edit: one more thing. I also tried it out correctly by calling it only once and before the KtxAsync.launch block. I think this is the real use case for this method and it worked!

Yup, glad you worked it out. :)

@czyzby
Copy link
Member Author

czyzby commented Apr 10, 2020

@Quillraven Does this mean that AssetStorage API is sufficient to replace AssetManager? Do you think anything needs improvements, including the documentation?

I believe I've documented the issue that you encountered in both README, as well as the exception class and loadSync docs.

@Quillraven
Copy link
Contributor

In my opinion yes. I merged it in master and created a new release. Will also use it from the beginning in new projects.

Documentation and example codes looked good to me. You definitely need some basic knowledge about coroutines but once you have that your documentation is clear.

@czyzby
Copy link
Member Author

czyzby commented Apr 10, 2020

@Quillraven Thanks again for your review, it was very helpful. I believe this branch is ready for merging. I'll try to publish a beta release later today.

@czyzby czyzby merged commit fc4431d into develop Apr 10, 2020
@czyzby czyzby deleted the feature/assets-async branch April 10, 2020 18:23
@czyzby czyzby changed the title Asynchronous coroutines-based asset manager: AssetLoader Asynchronous coroutines-based asset manager: AssetStorage Apr 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assets Issues of the ktx-assets module async Issues of the ktx-async module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite AssetStorage to the new ktx-async API
2 participants