Skip to content
New issue

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

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

Already on GitHub? # to your account

Update three-to-ammo dependency to new c-frame fork #60

Merged

Conversation

Elettrotecnica
Copy link

See #59

package.json Outdated
@@ -45,7 +45,7 @@
"ammo-debug-drawer": "^0.1.0",
"cannon-es": "^0.9.1",
"patch-package": "^6.5.0",
"three-to-ammo": "^1.0.1",
"@c-frame/three-to-ammo": "^1.0.3",
Copy link
Member

@diarmidmackenzie diarmidmackenzie Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just published 1.0.2. Not sure what 1.0.3 is?

https://www.npmjs.com/package/@c-frame/three-to-ammo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - just seen c-frame/three-to-ammo#5 & now understand. I will merge & pubish that and then come back to this.

@diarmidmackenzie
Copy link
Member

To get this to build, I had to update references in ammo-body.js and ammo-shape.js to:

const threeToAmmo = require("@c-frame/three-to-ammo");

Not sure whether that's the best fix, or if there's some way to alias three-to-ammo to @c-frame/three-to-ammo in package.json, which might be a more elegant solution?

@Elettrotecnica
Copy link
Author

Sorry for not checking thoroughly, should be fine now, I have used an alias so that the logical dependency does not change

@diarmidmackenzie diarmidmackenzie merged commit 874ad96 into c-frame:master Apr 29, 2024
0 of 6 checks passed
@diarmidmackenzie
Copy link
Member

Thanks for this. Do you need me to publish an update to npm?

@Elettrotecnica
Copy link
Author

Thanks for this. Do you need me to publish an update to npm?

three-to-ammo 1.0.3 is the latest and it seems you have published it. I think we are all set! :-)

Thanks a lot!

@Elettrotecnica
Copy link
Author

Concerning this repo, personally I do not use this from cdn, but maybe a release wouldn't be bad.

I could look into it if you prefer.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants