Skip to content

Conversation

@TimKraemer
Copy link

@TimKraemer TimKraemer commented Nov 22, 2023

Motivation:

The primary goal of this pull request is to enhance the CLI functionality within a Node environment. This initiative was prompted by challenges encountered due to the current inclusion of modified versions of GLTFLoader and DRACOLoader in src/bin. These versions, being outdated, have led to issues, particularly in generating correctly structured JSX.

Proposed Changes:

Refactoring GLTFLoader:

To make it work I just had to remove one line in https://github.com/pmndrs/three-stdlib/loaders/GLTFLoader.js:
const URL = self.URL || self.webkitURL

I assume that URL already exists in node. This probably needs a condition to not break browser use...
Unfortunately I'm not speaking typescript and don't understand enough of the project to create a pull-request directly in three-stdlib.

Adapting DRACOLoader:

Addressing the DRACOLoader was more complex due to extensive browser dependencies. As a workaround, I incorporated the DRACOLoader from Brakebein/node-three-gltf. This is a temporary measure until DRACOLoader in three-stdlib is optimized for Node/CLI usage.

Package Updates:

I have also updated various packages to their latest versions. Note: The update excludes prettier. If these updates cause any conflicts, they can be excluded from this pull request.

Caveats and Future Work:

  • My limited proficiency in TypeScript and incomplete understanding of the entire project scope have restricted my ability to directly contribute to three-stdlib.
  • The current state of this pull request achieves the intended functionality, but there is room for improvement in code quality.
  • Future revisions should ideally integrate DRACOLoader directly from three-stdlib, post its adaptation for Node/CLI environments.

Conclusion:

This pull request is functional and addresses the immediate issues with CLI functionality in a Node environment. However, it needs further refinements and potential contributions to three-stdlib for a more integrated approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant