Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About this PR
In order to make file-importing maintainable and readable, I created
FileImporter
, and bunch of file handler classes(in file/handlers/) instead of using single giant fileLoader
The end goals
File-related utils all goes to single place FileUtils, easier to maintain:
Adds introspection of supported file extensions, i.e. no hardcoded list, easier to maintain:
Adds FileManager, an abstraction of files cache, can normalize all file-import paths by building an artificial FileManager from
FileList
(File>Import),DataTransferItemList
(Drag n Drop), orunzipped File[]
(when user inputted a ZIP via File>Import or DnD), easier to maintain.Each file handle in standalone class, easier to read and maintain, in particular when searching symbols, fewer irrelevant matches.
QA
Why introduce new class instead of improving Loader
The name 'FileImoprter' is chosen because it has better semantic, i.e. files will be imported to the scene, instead of 'Loader' which only loads file into memory and not add to the scene.
Why file handler in (file/handlers) has FileHandler_XXX name pattern
Because some file extensions start with a number, for example
.3ds
, so instead of aliasing likeThreeDSFileHandler
, simply putting the ext name at the tail,FileHandler_3DS
.Why only one `import()` method in FileImporter
In Loader, there're 3 methods, one for loading single file, one for loading multiple files, one for loading DataTransferItemList.
However, there's no use cases of loading single file: file input
.files
isFileList
, drag n drop yields DataTransferItemList, so FileImporter has only one import methodimport( files )
which coversFile[], FileList, and DataTransferItemList
.Why file handler is a class not a function
More intuitive comparing to function interface, as you can see using functions required passing many arguments, some is just stub values in order to satisfy the fn interface. In contrast, FileImporter will delegate file-import to specific handler, e.g. call
FileHandler_GLTF.import(file)
to handle a gltf file, i.e. just thefile
is passing around, no stub values.Easier to extend for the future, for example, file handler
FileHandler_GLTF
may implementsexport(object)
, and then FileExporter delegates file-export of gltf to it.Whats the value of .instantiate() in FileHandler (base class)
.instantiate()
is simple and is 'great to have' because:import Loader + new Loader
..instantiate()
plus other FileUtils creates a flat import() flow, it helps debugging, in future PR, try/catch is made to trap parsing error and showing proper UI feedback:What is FileManager
FileManager is just a LoadingManager, it abstracted "files cache", When calling
FileImporter.import( files )
, a FileManager can be provided in the 2nd parameter, in this case, FileImporter will try 'loading files from the "file cache"' before tryingfetch
files from the internet.This mechanics normalized different ways of file-import, via creates a artificial FileManager for loaded files, while these files may come from file input, drag n drop, or even extracted from a ZIP:
editor.fileImporter.import( files )
(changes included in this PR)editor.fileImporter.import( files )
(changes included in this PR)fileImporter().import( files )
(changes included in this PR)i.e. With FileManager, users can import a ZIP of any supported file formats, as long as the corresponding loaders honors
loadingManager
.Alternatives of Loader.getSupportedFileFormat
Accepted file format is now
FileImporter.getAcceptedFileExts()
, extension includes dot e.g. '.md2' not 'md2', so Menubar.file.js needs a subtle change (included in this PR)Details
LoaderUtils
(fn)
createFilesMap
: no needed, implemented in FileManager(fn)
getFilesFromItemList
: moved to FileUtils (now returns Promise)Loader.js
(utils)
handleJSON
: moved to FileHandler_JS(utils)
handleZIP
: no needed, rewrote in FileHandler_ZIP(utils)
createGLTFLoader
: moved to FileHandler_GLTF(static)
getSupportedFileFormats
: no needed, usesFileImporter.getAcceptedFileExts()
instead(method)
loadItemList
: no needed, usesnew FileImporter().import()
instead (coveredDataTransferItemList
)(method)
loadFiles
: no needed, usesnew FileImporter().import( files )
instead(method)
loadFile
: no needed, usesnew FileImporter().import( [ file ] )
insteadFileImporter and FileManager
file/FileManager
is-a LoadingManager; act as files cache store, an abstraction of
file/FileImporter
import(..)
: imports files to editor (delegates to specific handler)canImport(..)
: check if any handler can handle the filegetAcceptedFileExts()
: arr of accepted file exts, ext incl dot, e.g '.md2'FileHandler, FileHandler_XXX and FileHandlers
file/handlers/FileHandler:
instantiate()
simplifiedimport + new
file/handlers/FileHandler_XXX:
import(file)
returning promise.file/handlers/FileHandlers:
getImportHandler(..)
to get proper handler for given filePreviews
Preview: https://raw.githack.com/ycw/three.js/editor-new-import-export-system/editor/index.html
Code: https://github.com/ycw/three.js/tree/editor-new-import-export-system/editor/js/file
#28324 (comment) Solved.