LDrawLoader: fileMap should exist when parsing.

[gero3]

Description
'fileMap' should exist when parsing the LDRAW model. This also removes a TODO from the editor.

[mrdoob]

Thanks!

[mrdoob]

Oh, maybe @yomboprime remembers why it was set to null by default?

[yomboprime]

Oh, maybe @yomboprime remembers why it was set to null by default?

This comment says it clearly:
// This object is a map from file names to paths. It agilizes the paths search. If it is not set then files will be searched by trial and error.

I'm afraid we've lost the ability to load directly from original parts repository files (instead of using the packed models). It is a pity I didn't make any app that used that functionality. Its use case is for example a Lego editor. Though I guess it is not a great problem overall.

[yomboprime]

I would add that great part of the overall complexity of the loader is to handle the various subdirectories and filename variations the format has. That complexity could be removed and rely solely on packed models.

[yomboprime]

I would add that great part of the overall complexity of the loader is to handle the various subdirectories and filename variations the format has. That complexity could be removed and rely solely on packed models.

Well I've read the code again and it is not that much complexity.

[yomboprime]

Oh, maybe @yomboprime remembers why it was set to null by default?

This comment says it clearly: // This object is a map from file names to paths. It agilizes the paths search. If it is not set then files will be searched by trial and error.

I'm afraid we've lost the ability to load directly from original parts repository files (instead of using the packed models). It is a pity I didn't make any app that used that functionality. Its use case is for example a Lego editor. Though I guess it is not a great problem overall.

Perhaps we should make this change, adding a check for fileMap (it is valid to have null fileMap):
if ( scope.fileMap && scope.fileMap[ fileName ] ) {

I think this would restore the functionality I was talking about (ability to load from the repository rather than packed models)

[gkjohnson]

Perhaps we should make this change, adding a check for fileMap (it is valid to have null fileMap):

Why is it valid to have a null fileMap? From looking at the code it doesn't seem to offer a difference in behavior?

I think this would restore the functionality I was talking about (ability to load from the repository rather than packed models)

For what it's worth the way I load models using the repository of parts is like so. I've uploaded the ldraw library to a personal repo specifically so I can test it. With this approach no file map is needed though it does try to load the same file multiple times until the right directory is found. I believe if you put all parts in the same folder it wouldn't have to check multiple directories:

// initialize library paths and materials
const loader = new LDrawLoader();
loader.setPartsLibraryPath( 'https://raw.githubusercontent.com/gkjohnson/ldraw-parts-library/master/complete/ldraw/' );
await loader.preloadMaterials( 'https://raw.githubusercontent.com/gkjohnson/ldraw-parts-library/master/colors/ldcfgalt.ldr' );

// load the model
loader.load( 'https://raw.githubusercontent.com/gkjohnson/ldraw-parts-library/master/complete/ldraw/10030-1%20-%20Imperial%20Star%20Destroyer%20-%20UCS.mpd' );

[yomboprime]

Why is it valid to have a null fileMap? From looking at the code it doesn't seem to offer a difference in behavior?

An editor/part browser that loads parts dynamically (by user action) could not have a file map. But it could be generated prior to loading. So it is not a big problem but the use case is there.

I believe if you put all parts in the same file...

Do you mean directory here, or packed file? I guess the former.

Thanks for the clarification about loading from the repo, BTW.

[gkjohnson]

An editor/part browser that loads parts dynamically (by user action) could not have a file map. But it could be generated prior to loading. So it is not a big problem but the use case is there.

I think this use case is still supported even if the fileMap field is defaulted to {}.

Do you mean directory here, or packed file? I guess the former.

Oh yeah I meant type "folder" 😁 thanks for the catch.

[yomboprime]

I think this use case is still supported even if the fileMap field is defaulted to {}.

Oh, you're right.