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 math utils issue #532

Merged
merged 6 commits into from
Apr 4, 2023
Merged

Fix math utils issue #532

merged 6 commits into from
Apr 4, 2023

Conversation

kalwalt
Copy link
Member

@kalwalt kalwalt commented Mar 28, 2023

⚠️ 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?

continue from PR #523

Can it be referenced to an Issue? If so what is the issue # ?
Yes see PR #523 and issue #515
How can we test it?

Test the Aframe and Threejs examples included in the repository, in the console it should print Aframe version 1.3.0 and no errors about MathUtils...
Summary

For more infos refer to the aforementioned issue and PR.
Does this PR introduce a breaking change?
No
Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser
Tested all the aframe examples.
Other information

Todo list:

  • upgrade aframe version in all examples
  • upgrade Threejs npm package version to 150 and three.min.js, three.js included in the examples.
  • upgraded GLTFLoader.js to version r147. (to fix issue with nft example see 0c70f71)
  • upgrade three version for the examples
  • update github action scripts

@kalwalt kalwalt added bug Something isn't working enhancement New feature or request labels Mar 28, 2023
@kalwalt kalwalt self-assigned this Mar 28, 2023
@kalwalt
Copy link
Member Author

kalwalt commented Mar 28, 2023

f364a57 fixed issue #530

- upgraded pm and vendor libs
- rebuild libs
@kalwalt
Copy link
Member Author

kalwalt commented Mar 28, 2023

I upgraded three.js npm package to 150 and the three.js/examples/vendor/three.js/build/three.min.js and three.js/examples/vendor/three.js/build/three.js to the same version (the prreviuos was 132). I didn't find any issue upgrading three to this version, Aframe shouldn'tbe affected because use his internal version.
I got some warnings about getInverse() deprecation, in the test-runner.html and arjs-session.html examples, those calls was inside three.js/src/markers-area/arjs-markersareacontrols.js and three.js/src/markers-area/arjs-markersarealearning.js. I will left some comments in the code for clarity.

@nickw1
Copy link
Collaborator

nickw1 commented Apr 1, 2023

Looks fine - happy to merge if all working as expected.

@kalwalt
Copy link
Member Author

kalwalt commented Apr 1, 2023

Looks fine - happy to merge if all working as expected.

Yes, i will do i have done only tests on desktop, i will do on Mobile soon, if all goes fine i will merge it.

@kalwalt
Copy link
Member Author

kalwalt commented Apr 4, 2023

updated GLTFLoader.js to r147 and partially fixed an issue (caused by changes in three.js r150). We can't use the newer because it require to import AR.js in a <script type="module">, but we don't have this feature yet. It's not a big deal to provide a jsm version of the lib but definetetly not the goal of this PR.
Note that in the three.js/nft example you will receive this warning:
three.min.js:7 THREE.PropertyBinding: Trying to update node for track: mesh_0.morphTargetInfluences but it wasn't found
I think it's not crucial we can skip for now.

@kalwalt kalwalt merged commit e3a06c3 into dev Apr 4, 2023
@kalwalt
Copy link
Member Author

kalwalt commented Apr 4, 2023

@nickw1 i will make a new release as i think this is an awaited fix.

@nickw1
Copy link
Collaborator

nickw1 commented Apr 5, 2023

@kalwalt I agree - sorry only just seen your update, I see you;ve released anyway which is great.

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.

2 participants