search

AR-js-org / AR.js

Image tracking, Location Based AR, Marker tracking. All on the Web.
MIT License
5.3k stars 909 forks source link

Mathutils #525

Closed DougReeder closed 1 year ago

DougReeder commented 1 year ago

⚠️ 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? Updates A-Frame to v1.3.0 and uses the name MathUtils instead of Math, to resolve some lingering incompatibilities

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

515

https://github.com/AR-js-org/AR.js/issues/515 incorporates https://github.com/AR-js-org/AR.js/pull/517

How can we test it?

I've run all the examples, and there are no Math/MathUtils errors.

Summary

  1. Math.sin(), etc. is the JavaScript builtin Math, while Math.degToRad() and Math.radToDeg() is three.js. Recent versions of three.js changed the name to MathUtils, to reduce the confusion.
  2. three.js v 0.146.0, the current dependency, uses the name MathUtils.
  3. A-Frame 1.0.4 is three years old, unsupported, and references the three.js methods using the name Math, so the dependency really ought to be updated

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 Examples run under MacOS 13.2.1, Firefox 110.0.1 (64-bit)

Other information

kalwalt commented 1 year ago

There is already a PR for this why open a new one?

DougReeder commented 1 year ago

Closed in favor of https://github.com/AR-js-org/AR.js/pull/523 (albeit, the A-Frame version should still be updated).