Discussion: Separate asset loading from render classes like Sprite etc | Cleanup

[erf]

Ok, so this is a potential touchy subject and big breaking changes, but i think it is worthy a discussion ( for me at least ;) .

Asset vs rendering

Now loading assets happens asynchronously after you pass a named asset as a constructor parameter, like in Sprite, SpriteSheet, etc. I will argue why this might not be the best solution.

Loading assets this way, which is an async task, makes you have less control over asset loading as it is split over many classes, and you might not know when things are finished loading. If you have a single place to load a list of assets, you could load things in parallell and you would have more control over when things are done.

Also there is a matter of separation of concern; now you have a class which handles both rendering and asset loading. As a class do more stuff, it makes for more problems. You have to do validation / null checks to make sure the assets are loaded, and you pull inn more dependencies and lines of code per class.

It might be convenient to pass an asset name to a class like that, but i think you could avoid problems and make things more robust and more performant, if we split up asset loading from rendering classes and rather pass the loaded assets to the constructors. Like Image to the Sprite class.

Separate Asset helper classes could help with this, which you would call at initialization or before some level etc.

Cleanup

Also, as you mention Flame is a minimal engine, i think there is room for trimming down even more and clean up some APIs, and perhaps move out some stuff as third party libs. For example, not sure if NineTileBox should be here as it seem very spesific? You also have a bunch of particles classes which is pretty spesific, like rotating_particle,scaled_particleetc. Also theSprite` class have 5 different render methods, could we combine them and rather have more optional named params?

These are potential big changes, so not sure what kind of policy you have about making breaking changes like this, or if it would be interesting at all, but i would like to hear your opinion!

Cheers!

[erickzanardo]

This has been already been mentioned some times. And I always think is worth discuss about.

About the loading, I like to have the both options, I think it is very convenient to have the auto loading feature, it is particular useful when doing small games to be integrated alongside normals, for gamification for example.

But it is also very importante to let the develop also handle loading themselves, that is why methods like this exists. I am aware that not all classes have methods like this, but I think we should focus on fixing that.

About particle systems and thinks like that. The minimalist idea, is that you can use or not the resources from Flame, and since Flutter has tree shaking, there is no problem on you having code on Flame that your game does not use. We normally draw the line to make a feature a separate package, when that feature demands native code, or if that is really an edge case. I don't really think that the NineTileBox is such a specific use, as it is regarding building UIs, and I think that is pretty common on games.

That are my thoughts, let me know what you think :)

[erf]

Thanks for the comment :)

I understand the need for supporting both ways of doing it, and the convenience of it. I might also be a matter of taste. I did not know you also had a method for Sprite.fromImage (still learning), which is nice. However in doing so, you also bring in more complexity, and more choices for the user, and more things to document, which can make it overwhelming, and make the user of Flame, less productive.

Regard minimalisme, there are degrees of it, and maybe i'm a bit preoccupied in details sometimes. Have not really looked into NineTileBox, but i would think Flutter has a lot of ways for building UIs?

Regarding the auto loading feature; should user be allowed to use Sprites etc, when you dont know if the asset has been loaded? I mean like, call a method on the sprite and have it returned null? You could do null checks, but these null checks seem really tiresome, i would rather the app fail if that was the case, than make some invalid logic.

Thats my thoughts :)

[erickzanardo]

However in doing so, you also bring in more complexity, and more choices for the user, and more things to document, which can make it overwhelming, and make the user of Flame, less productive.

That is true, but IMO, our faulty here is the lack of docs and tutorials, we should have better docs that would make it easy for the developer to choose the best feature that serve their needs.

Regard minimalisme, there are degrees of it, and maybe i'm a bit preoccupied in details sometimes. Have not really looked into NineTileBox, but i would think Flutter has a lot of ways for building UIs?

Sure, and we encourage devs to use Flutter to build UIs, see the widgets overlay API. But some UIs have some details really related to games, and sometimes what Flutter provides is not enough, so for that cases, we try to provide some utility widgets/classes that can help that, especially in retro looking games, NineTileBox can be really helpful.

I also would like to try and summon @luanpotter here, and see his thoughts as he is the original author of these APIs 😉

[erf]

Ok, here is a suggestion to support both construction of rendering objects with assets and also from name:

Let all rendering classes support pre-loaded assets ( e.g. Image ), in the constructor by default.

Make extensions per render class, which implements a «withAsset» static function which takes a asset name, and returns a future of the class obj, with a loaded asset. This way you can grab a list of futures an parallelize the asset loading. And you don't have to worry that the asset might be null.

[luanpotter]

I like having a way to pre-load everything. My goal was to provide that with methods like Sprite.fromImage. maybe we sometimes forget to add convenient apis, I'm super open to PRs to fix that.

about the existing lazy loading apis, I'm not sure, I do think they have value. As a first step we should make it very nice and easy to do the eager loading, and depending on how it is we can consider deprecating the lazy loading apis later.

what do you guys think? open to PRs to address the current lackings with eager loading :)

[erf]

I'll close this now. I just felt like pitching some ideas and helping out a little bit, since i've been working with Flutter lately, and do have a little game dev experience, but i'm not really doing any game dev with Flame right now. I think maybe doing some experimentation on my own might suit me better right now, as i have a few ideas which will probably be difficult to integrate in Flame without breaking stuff, and i can be quite particular about details. Later, guys :)

[erickzanardo]

Thanks for all the contributions @erf , we will try to incorporate your suggestions with time.

Hopefully you will get back to Flame eventually and make something awesome :D

[erf]

Been doing some experimentation -> https://github.com/erf/flim