Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#459 fix: Update default draco decoder path #461

Merged
merged 3 commits into from
Jan 10, 2023
Merged

#459 fix: Update default draco decoder path #461

merged 3 commits into from
Jan 10, 2023

Conversation

afonsobspinto
Copy link
Member

@afonsobspinto afonsobspinto commented Dec 20, 2022 •
edited
Loading

closes #459

  • Add prop to define draco decoder path
  • Changes default draco decoder path to cdn recommended by draco

@afonsobspinto afonsobspinto added the bug Something isn't working label Dec 20, 2022
@afonsobspinto afonsobspinto added this to the Patch milestone Dec 20, 2022
@afonsobspinto afonsobspinto removed the request for review from zsinnema December 26, 2022 15:00
@afonsobspinto afonsobspinto changed the title #459 fix: Set draco decoder path to local node_modules #459 fix: Update default draco decoder path Dec 26, 2022
@afonsobspinto afonsobspinto mentioned this pull request Dec 26, 2022
Merged
@afonsobspinto afonsobspinto linked an issue Dec 27, 2022 that may be closed by this pull request
Remove tight dependency on ThreeJS repo for draco loader #459
Open
@enicolasgomez
Copy link
Contributor

@afonsobspinto @ddelpiano I'm ok with changing the url dependency on the 007 branch but for development, I though we would be managing this here?

https://app.zenhub.com/workspaces/geppetto-meta-6047b13d88e7e70013d4589b/issues/gh/metacell/geppetto-meta/459

Last thing we've discussed is using something similar to CopyWebpackPlugin so files are grabbed from Draco's loader installation directory directly as we saw that files were there

@afonsobspinto
Copy link
Member Author

afonsobspinto commented Jan 3, 2023 •
edited
Loading

@enicolasgomez as commented in the issue you linked, relying on webpack doesn't look like the correct solution to me since that's a configuration that happens at the application level and not on geppetto.
We should't have to force all applications that use geppetto to configure something with webpack. The library should work out of the box no matter the bundling strategy

@enicolasgomez enicolasgomez merged commit ecce8b3 into development Jan 10, 2023
@ddelpiano ddelpiano deleted the feature/459 branch January 18, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddelpiano ddelpiano ddelpiano approved these changes

@enicolasgomez enicolasgomez Awaiting requested review from enicolasgomez

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
Patch
Development

Successfully merging this pull request may close these issues.

Remove tight dependency on ThreeJS repo for draco loader
3 participants
@afonsobspinto @enicolasgomez @ddelpiano