Assets

[andretchen0]

Assets are currently loaded in a sort of roundabout way.

assetLister.js

Runs at compile time, outputting src/assets.js: a JS object containing a representation of the directory structure and files of /assets.

assetLoader.js

loads assets.js at run time and exposes their paths via getURL(str). But getURL(str) doesn't take the asset path from src/assets.js. Instead, it wants that path, minus assets/ and minus the file extension.

Phaser.load.image

Phaser needs to know about images at run time. This largely repeats src/assets.js. But using getURL urls from assetLoader, e.g., this.Phaser.load.image('input', getUrl('interface/hex_input'));.

What's more, Phaser.load.image needs a key as a first argument, but the provided key is often different than the name of the image.

E.g., Load like this: this.Phaser.load.image('input', getUrl('interface/hex_input'));

Use it like this: this.display.loadTexture('input');

So a dev who's looking at the second line in code somewhere needs to figure out that the image is actually called hex_input. They can search, of course. But if the key is a common string in the codebase, as it is here, it's difficult to find the actual image name.

Each step adds a little extra complexity. Data is duplicated.

Proposal

Delete assetLister.js and assetLoader.js and assets.js

Most of the image assets are already written down in src/game.ts. In some cases, it appears they just need to be written out explicitly in order to be picked up by Webpack and bundled.

Split HTML / Phaser images?

Doing this would allow us to load all the Phaser assets into Phaser without needing to write them down, assuming we changed the filenames to keys.

E.g., if the Phaser key is supposed to be 'input', the image should be called 'input.png', not 'hex_input.png'.

---

Just some ideas. Any thoughts?

[DreadKnight]

Very valid points/concerns. The keys are indeed annoying overall, as they're forced afaik, instead of being purely optional.
We could make all the changes you listed. If I think about it, we won't need an asset lister that much anyway after we migrate to Phaser 3 and load assets on the fly as of #1584; and regarding the html file, the source index should definitely be split as of #1111, provided you didn't mean the one from the "build" folder itself.

[andretchen0]

Related:

I don't currently see any Webpack-specific code in the game's modules.

• Is that a choice? – maybe to allow switching bundlers in the future.
• Or is it simply that no one found a need for Webpack-specific code?
• Or something else entirely?

Any ideas?

I ask because Webpack has its own require that allows searching a directory at compile time and making the results of the search available at run time.

This is basically what assetLister.js is doing, except require.context built in to Webpack and doesn't require extra code or build steps, etc.

E.g., putting this in game.js in the constructor will list all the paths of png files in ./assets whenever a new Game is created:

console.log(require.context('../assets/', true,  /\.png$/).keys());

[DreadKnight]

@andretchen0 Sounds good! Plan was to use Webpack from the get going from my point of view but either some of the features weren't there at the time or whomever implemented things lacked some experience/knowledge, who knows...
It's good I guess that you question things, but when they're done poorly, it's probably because someone couldn't do them better (at the time at least) or didn't knew how back then, plus years went by, so now there are more and better choices.

[andretchen0]

either some of the features weren't there at the time or whomever implemented things lacked some experience/knowledge

No prob. It's pretty common just to poke at build configs until things work and not ask too many questions.

I don't mean to suggest someone didn't know what they were doing when I ask about why a decision was made. I just want to know if I'm about to step into something: Sometimes there's knowledge out there – "We did this non-obvious thing because the obvious thing had these problems for this project ..."

But sometimes things are the way they are just because they were good enough. And that's great, because it means we can go ahead and make them better.

[DreadKnight]

Figured it's what you meant, I totally get it, no worries. I'll try to give shorter answers next if we bump into similar scenarios.

[andretchen0]

@DreadKnight

This is mostly finished with the last PR I opened. One thing I haven't done is streamline loading of assets into Phaser. I'd like to do that as it took me a lot longer than necessary just to put a simple image on screen in Phaser:

• Put the image somewhere in ./assets/.
• Figure out the 'key'.
• Send the key to assets.getUrl(key).
• Send that to Phaser.load.image(cache_key, assets.getUrl(key)).
• And only then can you put the image on screen using the cache_key.

Especially in game.ts, there's a bunch of code like this:

this.Phaser.load.image('hex', getUrl('interface/hex'));

Ideally, we wouldn't need to write that out at all. If we just decide on an organization, the new assets.ts can just load everything into Phaser with a single function call.

Proposal

• Add a directory to the top level of ./assets/ called phaser/autoload/.
• Any assets in that directory with a png, jpg, gif extension will be automatically passed to Phaser.load.image(key, asset_path) when assets.usePhaser(phaser:Phaser) is called.
• Phaser requires a key for assets. I'll simply use the filename – no extension. Since 2 files in different directories can have the same filename, I'll have the autoloader throw an error if a key is reused.

Blue sky proposal

Regardless of path, all assets should simply be accessible by filename minus their extension. E.g.,

/assets/interface/hex_input.png // key == "hex_input

But ...

/assets/images/backgrounds/dark_forest/bg.jpg // key == "bg"
/assets/images/webpage/bg.png // key == "bg"

... will not be allowed because both of their keys are bg. We'll just have the system throw an error on build to enforce the decision.

Right now, to get the key, you remove /assets/ and the extension from the file's path. But if we move/rename any of the folders the assets are in, we're going to break stuff.

[DreadKnight]

Sounds good, overall, we really need some streamlining/simplification, so all assets could be autoloaded for now, except when we'll just load required unit cardboards/spritesheets #1584 #678 #1692

• regarding proposal: we could drop the phaser/ part from autoload directory
• regarding blue sky proposal: the backgrounds aren't in final form, some will be more modular (layers), allowing animations and perhaps some parallax effect given time, so until then, we could rename the bg to the asset, but for now we could just have them within a single folder, like .../backgrounds/dark_forest.jpg for example

[andretchen0]

regarding proposal: we could drop the phaser/ part from autoload directory

A particularity of this project is that we've got assets split between HTML and Phaser. The Phaser stuff has to be loaded, then fed to Phaser. We don't want to send assets that are only displayed in HTML to Phaser.

I'd like to make sure that's obvious to someone new to the project. "Everything in this directory will be sent to Phaser!!"

I'll throw a readme in there as well. And maybe write a small CI check to ensure unique keys.