AssetLoader and generics

Hi!

Thanks for the great engine. While using it, I just wondered why there are no generics in the AssetLoader interface? The load just returns an object. Like so:

public interface AssetLoader<T> {

    public T load(AssetInfo assetInfo) throws IOException;

Just a n00b question.

Work it through… how would you register this in such a way that AssetManager.loadAsset() would pass the type through?

Can’t think of a way right now :slight_smile: Thanks

The naive question is always … a hint of something. :smile:

Let’s say user want to have a load method with generic something:

<T> load(AssetInfo info, Class<T> clazz);
or
<MyAsset> loadMyAsset(AssetInfo info);

Why?

  1. Because AssetInfo match by extension. So .xml, .json for example will cause trouble. The job to distinguish serveral kinds of .xml, .json, be handled to AssetLoader which also return Object.
  2. Because you can not really directly extends AssetManager, because you have to bypass JmeSystem. or call setAssetManager in awkward way.

These two reasons led me to folk current Asset system to make them more generic in my code base. Also sometime I want to inject the AssetManager…The current design’s quite “closed” in my POV, don’t really try to find a work around it through… So If there is a

AssetLoader<T>

… may be better workflow, anyone?

    public <T extends Asset> T loadAsset(AssetKey<T> assetKey);


    interface AssetKey<T extends Asset> {

    }

    interface Asset {

    }

would be my ‘generic’ base.

Then how do loaders get registered in such a way that you could case the arguments to the generic type? How are they looked up in a registry?

The only way I can possibly make it work is if asset loaders are registered for the class they produce (not the generic type but the actual java.lang.Class) and the AssetKeys also specifically know what java.lang.Class they will be loading. All to avoid a cast in the loader.

The only way I can possibly make it work is if asset loaders are
registered for the class they produce (not the generic type but the
actual java.lang.Class) and the AssetKeys also specifically know what
java.lang.Class they will be loading. All to avoid a cast in the
loader.

Yes. Why not?
Let AssetKeys know about its return results instead of just a path is really considerable. What I do is to extend AssetKey with AssetKeyParameter (hints).
AssetKey will tell its Loader (not nessesary AssetLoader) if it have any hints to smooth the process. In my asset pipeline, a lot of time I just want to extract piece of information from j3o, json or xml.

I prefered more parameters or at least a custom way to let AssetLoader know about its result form. If we not affraid of TypeToken
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/reflect/TypeToken.html
It’s can be a possible candidate to make class thingy useful to become a Hint.
And this is simpliest needed registry.

Anyway, I don’t think one should touch too much into the Asset core, because this require Java6 at some point to support generic features. :slight_smile: That’s why a folk.

Never said it is worth it. :wink:

But i had a code reviewer once that said this ‘unnecessary casts’ (his words) are nearly the same as a not working code.
Generics nightmare

I think we all need to help people move away from Java 5, 6 and 7… It’s all old news and Oracle will only keep releasing public updates until about March this year for Java 7.

Going back to the original question, why would you really need AssetLoader to be generic? When you end up actually loading assets you mostly use the assetManager.loadAsset() method or one of it’s siblings and as far as I can tell you can use that without having to worry about casts.

assetManager.registerLoader(CustomObjectLoader.class, "custom");
final CustomObject customObject = assetManager.loadAsset(new AssetKey<>("customs/object.custom"));

Generics and load by class type instead of extension is not necessarily the same thing. Adding generics to the AssetLoader would mean that developers implementing their own custom loaders would force themselves to return the correct type and also make unit testing a bit easier although they could just set it to Object and be back to square one.

On the other hand you could enforce assetManager.registerLoader() to take the loader class as well as the expected return type and extensions. This just complicates it though and doesn’t really provide much benefits inside the AssetLoader.

But how would you register it by type such that you could look up the loader for some type… you can’t unless there are concrete classes to deal with… which you can’t get from generics.

You’re not really doing lookups by the generic stuff as generics are removed at compile time. When you register a loader you would have to pass in a type that you would like to use for reverse lookup, but like I said I’m not sure that would be needed at all.

Then there is no reason for the loaders to return anything other than Object… they can return anything they want anyway. If we want to do unsafe casting then the asset manager can do that without involving the loaders. I thought we’d already done it in fact.

Yep: T loadAsset(AssetKey key)

Yes that’s what I meant actually. The only reason to add generics to the AssetLoader is for developer peace of mind to ensure type safety of whatever it is they are doing and to make it easier to unit test.

But to avoid one (possible) cast in the AssetLoader we’d have to redo the asset manager registration and break all of the existing AssetLoaders (to do it properly). I just don’t think it’s worth the trouble, personally.

Yeah not that big of a deal. Just wondered when I implemented a few AssetLoaders.

We have used JAVA almost for 20 years now, an ages old platform that has never been rewritten fully. When adding generics and removing all the casts, hundreds of bugs where found and the coding got easier. So I automatically see a bug when there is a cast or an Object :smile:

loadAsset
T loadAsset(AssetKey key)
Load an asset from a key, the asset will be located by one of the AssetLocator implementations provided in the registerLocator(java.lang.String, java.lang.Class)
call. If located successfully, it will be loaded via the the appropriate AssetLoader implementation based on the file’s extension, as
specified in the call registerLoader(java.lang.Class, java.lang.String).

This sometime is not enough.
I usually have to do “hacky things” like this to load a Json file:
Prefix with some crazy tag and hack the locator

assetManager.registerLocator(GenericFileLocator.class,"::");
assetManager.registerLoader(GenericJsonLoader.class,".json_someJsonData");
assetManager.loadAsset("::realPath/myFile.json");

or

hack the AssetKey

assetManager.registerLoader(GenericJsonLocaltor.class.".json_someJsonData");
assetManager.loadAsset(new JsonAssetKey("myDataExtension"){
        String getExtension(){return ".json_someJsonData";}
        String getName(){ return nameWithout_someJsonData;}
});

I never got why people would suffix their files with xml content with .xml or json with .json. Nobody ends their binary files with .bin either… ^^

Yes, this is funny because you will run out of idea for extensions to name your file soon …:)) Sorry bad joke.

.json or .xml needed for other editors to recognize and edit them properly. Of course after go down JME pipeline, we can rename them but it add extra cost. Become more annoying if you have to go back and fort usually while editing those files.

I think improvement needed for the extension mapping if generic loader is too much problematic instead of gaining anything.

 protected static String getExtension(String name) {
int idx = name.lastIndexOf('.');
//workaround for filenames ending with xml and another dot ending before that (my.mesh.xml)
if (name.toLowerCase().endsWith(".xml")) {
idx = name.substring(0, idx).lastIndexOf('.');
if (idx == -1) {
idx = name.lastIndexOf('.');
}
}
if (idx <= 0 || idx == name.length() - 1) {
return "";
} else {
return name.substring(idx + 1).toLowerCase();
}
}

because this is pretty bad also!


Forget to answer the question about why I needed generic loader, let list down case by case:

  1. T loadAsset(AssetKey key); => None AssetManager nor AssetKey, AssetLoader know anything about T… It just cast Object to T and ignore if there is exception.
    jmonkeyengine/DesktopAssetManager.java at master · jMonkeyEngine/jmonkeyengine · GitHub

  2. AssetKey clone(); => Similar cast
    jmonkeyengine/AssetKey.java at master · jMonkeyEngine/jmonkeyengine · GitHub

In my POV, AssetKey is nearly useless even in fact it may help much more to “hint” the asset pipeline. I think T (Type) is too broad for a generic type but <? extend Object> is fair enough. Another thing, AssetKey abstraction are thick and assume a single object.
What about a AssetManager can load a whole pack by an AssetKey of Collection of AssetKey (but not return immediately):

AssetKey<Collection<AssetKey>>

and later

getAsset(new AssetKey<Collection<AssetKey>>("a model in a pack"));

or a Nested fashion?

The point of making AssetKey generic, I think because it should hint its mapped loader someway. For example a generic loader
will have to be matched and choosed for AssetKey that may bring much more control for the user.

assetManager.registerLoaderByType(JsonLoader.class, TypeToken.of(Json.class));
assetManager.loadAsset(new AssetKey(TypeToken.of(Json)));

In the past, Java class definition have to be loaded by classloader to get reference elsewhere. That situation cause overhead and Java developer make a presumption that using class type to determinate something (like as a key in a map) is “bad practice”. This is not totally true nowaday and not apply to AssetLoading also because we use java ClassLoader intensively already.

I have never seen an editor so bad that you can’t define how to open which file endings.

Deficient editor. Mine lets me set this after loading and even define the new mapping in like two clicks.

“This is hard because my tool is in the way…” is very sad. :slight_smile: