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.

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

Fix reference errors in THREE.Math and THREEx #523

Merged
merged 3 commits into from
Mar 28, 2023

Conversation

Hyeonoo-Park
Copy link
Contributor

Bug fix!

  1. THREE.Math was renamed to THREE.MathUtils in r113. THREE.PlaneBufferGeometry is deprecated.
  2. THREEx is not defined!

⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️

What kind of change does this PR introduce?
bugfix

Can it be referenced to an Issue? If so what is the issue # ?
Fixes #515

How can we test it?
See https://github.com/Hyeonoo-Park/react-ar-js-test sample project

Summary

Does this PR introduce a breaking change?
???

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser
Samsung Galaxy Note10+s(SM-N976N)
Chrome (110.0.5481.153)

Other information

@kalwalt
Copy link
Member

kalwalt commented Mar 11, 2023

Hi @Hyeonoo-Park I will review and will check the code in the next days when i have a bit of time.

@kalwalt
Copy link
Member

kalwalt commented Mar 15, 2023

Hi @Hyeonoo-Park I tested your changes, and as @DougReeder pointed out in this PR #525 the aframe version used in the examples need to be upgraded to 1.3.0 can you make these changes for all the examples (location, markers and nft) ?
second: we should upgrade also the aframe version in the package.json to the same version (1.3.0) and rebuild the libs. If you make these changes i will happy to merge it.

@kalwalt kalwalt added bug Something isn't working enhancement New feature or request labels Mar 15, 2023
Copy link

@DougReeder DougReeder left a comment

Choose a reason for hiding this comment

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

LGTM

@kalwalt
Copy link
Member

kalwalt commented Mar 21, 2023

All the aframe examples will need to upgrade the Aframe version but i can do this. If nothing against @nickw1 i will merge this.

POST-EDIT note: see this example is using aframe 1.0.4

<script src="https://aframe.io/releases/1.0.4/aframe.min.js"></script>
but we upgraded to 1.3.0 and probably will raise an error. But anyway is better to use the same version to avoid confusion.

@kalwalt kalwalt changed the title fix refrence erros in THREE.Math and THREEx Fix refrence erros in THREE.Math and THREEx Mar 21, 2023
@kalwalt kalwalt changed the title Fix refrence erros in THREE.Math and THREEx Fix reference errors in THREE.Math and THREEx Mar 21, 2023
@kalwalt kalwalt merged commit 1a66d11 into AR-js-org:dev Mar 28, 2023
@kalwalt kalwalt mentioned this pull request Mar 28, 2023
5 tasks
@kalwalt
Copy link
Member

kalwalt commented Mar 28, 2023

continue on #532

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants