upgrade three.js and aframe

[kalwalt]

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

THis PR will upgrade Three.js and Aframe dependencies.
Can it be referenced to an Issue? If so what is the issue # ?
See issue #611 #504 #563 #535
How can we test it?

Test all the examples
Summary

Newer Three.js lib need to imported as a module so probably we need to create .module.js version for each build libs. Doing this probably we solve also issue #504
Does this PR introduce a breaking change?
I hope no but this is a draft PR.
Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser

Other information

[kalwalt]

I should have upgraded most of the three and aframe location-based examples, The only issue that i found is related to Aframe i got this warning:
renderer.js:39 THREE.WebGLRenderer: The property .useLegacyLights has been deprecated. Migrate your lighting according to the following guide: https://discourse.threejs.org/t/updates-to-lighting-in-three-js-r155/53733.
but i think there is not so much that we can do for now.

[kalwalt]

@nickw1 now all the examples should use aframe 1.6.0 and three 164, i applied all the changes you suggested in issue #611, i would merge it before 7 or 8 december, of course if you have nothing against. I haven't found any particular issue but let's do some more testing with the examples...

[nickw1]

@kalwalt great! Will test the location-based examples and let you know if there's any problem but other than that, all should be good!

[kalwalt]

@kalwalt great! Will test the location-based examples and let you know if there's any problem but other than that, all should be good!

Thank you @nickw1! I have the same idea, probably there shouldn't be any issue. Just a quick question; do you have an idea because exists this file: https://github.com/AR-js-org/AR.js/blob/master/aframe/dist/main.js ?
I have made some fixes for the multiMarkers option in artoolkit5-js, not all the required methods are implemented yet but maybe after this PR i may concentrate to improve it.

[nickw1]

@kalwalt I don't think that main.js is needed, because the bundles are placed in build. At a guess it was something which was committed accidentally.

[nickw1]

@kalwalt have just tested the location-based examples.

Some comments:

• some of the old location-based examples (in location-based) do not work, but the older versions of the location-based API are not really being maintained anymore and they don't work on master either. Can we just remove this directory? It will ease maintenance effort for one thing, only the new-location-based components have seen any updates for several years.

• the new-location-based examples mostly work with this PR the same as they do on master. avoid-shaking doesn't seem to work on either. However the osm-ways example is non-functional with this PR, while it works fine on master. Let me see if I can investigate this.

[nickw1]

Update on the above: I can get osm-ways to work if the a-camera is placed after the custom component, i.e.

<a-entity osm></a-entity> <!-- this does not work if placed AFTER the a-camera. -->
 <a-camera gps-new-camera='gpsMinDistance: 5; simulateLatitude: 51.049; simulateLongitude: -0.723' position='0 20 0' wasd-controls='acceleration: 1300'></a-camera>

Not sure if this is related to an internal change in A-Frame?

[kalwalt]

Update on the above: I can get osm-ways to work if the a-camera is placed after the custom component, i.e.

<a-entity osm></a-entity> <!-- this does not work if placed AFTER the a-camera. -->
 <a-camera gps-new-camera='gpsMinDistance: 5; simulateLatitude: 51.049; simulateLongitude: -0.723' position='0 20 0' wasd-controls='acceleration: 1300'></a-camera>

Not sure if this is related to an internal change in A-Frame?

Yes, It could be an internal change in A-frame. I will test your suggested fix when i can.
In regards of the other location examples, maybe it's better to focus with one version/type. Anyway let me think few days about, as I haven't so much time this week end.

Post edit: briefly think... If the old location examples as you said doesn't work, neither in master, better to remove them, please can you do this if you have time?

[kalwalt]

For the A-frame examples I haven't developed a module version as I did for the three.jsexamples. simply because for the vanilla J's examples wasn't required, but maybe it potentially could solve some issues loading as a module in React for example?

[nwcourses]

(Sorry this is @nickw1, I logged in on a different account)

Some of the location examples do work, but they aren't using the new-location-based components so I'm thinking they are maybe misleading. So are you still good for me to remove them?

I will do so in this branch if it's OK, as then we don't get master diverging.

Might be worth developing a module version for A-Frame too.

[nickw1]

The problem in osm-ways is odd because my Hikar webapp (using AR.js) works fine with aframe 1.6.0 and AR.js with this PR, the only issue is a PlaneBufferGeometry vs PlaneGeometry error in one of its dependencies (my own aframe-osm-3d lib which I can quickly fix). And in the Hikar webapp the camera is before the entity.

For now I think I'll rewrite the osm-ways example using the same template as some of the (working) examples, there is probably some difference I haven't spotted just yet.

With the reduce-shaking examples I think they need a real mobile device, come to think of it. Let me try on one.

[nickw1]

@kalwalt a few updates:

• reduce-shaking works fine on a real mobile device.
• osm-ways also works fine on a real mobile device.

Related to the osm-ways issue, I have discovered an issue in which simulateLatitude and simulateLongitude do not generate the gps-camera-update-position event correctly if hard-coded in the HTML, though they do work when they are updated dynamically. This seems to be the case in master too, so maybe something which slipped in during a recent update.

However I am unlikely to have much time to look into this soon, so maybe it's best raised as a separate issue. I'm more likely to be focusing on locar.js in any case.

For the "old" location-based examples these seem to work fine:

avoid-shaking
click-places
max-min-distance
peakfinder-2d

The others appear to be using old coding standards.

What I propose in that case is to change the new-location-based example directory to location-based, move the four examples above to a classic-components subdirectory, and delete the rest. Does this sound good? Let me know if I can go ahead and do this.

It looks like there is nothing in this PR which stops the examples working, though.

[kalwalt]

@kalwalt a few updates:

• reduce-shaking works fine on a real mobile device.
• osm-ways also works fine on a real mobile device.

Related to the osm-ways issue, I have discovered an issue in which simulateLatitude and simulateLongitude do not generate the gps-camera-update-position event correctly if hard-coded in the HTML, though they do work when they are updated dynamically. This seems to be the case in master too, so maybe something which slipped in during a recent update.

However I am unlikely to have much time to look into this soon, so maybe it's best raised as a separate issue. I'm more likely to be focusing on locar.js in any case.

For the "old" location-based examples these seem to work fine:

avoid-shaking click-places max-min-distance peakfinder-2d

The others appear to be using old coding standards.

What I propose in that case is to change the new-location-based example directory to location-based, move the four examples above to a classic-components subdirectory, and delete the rest. Does this sound good? Let me know if I can go ahead and do this.

It looks like there is nothing in this PR which stops the examples working, though.

Those examples are not necessary, so much the better. Proceed as you suggested. As you finish I will implement the module feature for aframe as discussed.
In regards the issue you catch it for the gps-camera-update-position event, better to open a specific issue for kepp track of it.

[kalwalt]

I will do so in this branch if it's OK, as then we don't get master diverging.

Hadn't the time to answer you, yes of course, work on this branch!

[nickw1]

OK - done.

[kalwalt]

OK - done.

thank you @nickw1 if i have time i will work on this sunday evening, we are close to finish this task.

[kalwalt]

I'm testing an example in this gist but it's not loading properly, i will investigate it.

[kalwalt]

I'm not sure that the module aframe versions dist libs will be helpful. Let me explain, the AFRAME arjs system is heavily based on THREE dependencies (i'm not referring to the internal aframe super-three.js code) from AR.js internal API, this cause the multiple three.js instances error for example and a bigger file size, it' shouldn't be necessary but this was designed from the beginning (more or less), so what we can do now? we should redesign the whole code base, not for sure in this PR...