Merging upstream from XRPackage

[yahengsu]

Updated avatar files based on previous commits on the files in the XRPackage repo. Based on issue #36 on the XRPackage repo.

Changes in each commit are followed by the commit-msg and commit hash of the original commit in the XRPackage repo, with multiple commits being melded together if multiple changes were made to a single file. I looked into cherry-picking commits from the XRPackage repo to the exokit repo since it would be nice to maintain git history, but didn't find an easy way to do so.

Feedback on anything I missed and how to test the changes would be greatly appreciated! Apart from opening index.html locally, running npm start, and checking the files manually, I wasn't really sure how to confirm these changes were acceptable.

Also please let me know if I missed the mark completely and did something completely different than what you intended 😛 this is all a great learning experience for me!

[avaer]

Thanks!!

This looks correct on a first look -- we'll need to test it with some headsets to make sure it's working.

Once that is done we can probably move the xrpackage references to point to this repo.

[yahengsu]

Awesome, please let me know if there's anything I missed!

[avaer]

Unfortunately this did not load for me (opening index.html on the local server):

VM278 three.js:50948 Uncaught SyntaxError: Unexpected token 'export'
OrbitControls.js:10 Uncaught SyntaxError: Cannot use import statement outside a module
Reflector.js:4 Uncaught SyntaxError: Cannot use import statement outside a module
bmfont.js:11 Uncaught ReferenceError: THREE is not defined
    at Object.1../lib/utils (bmfont.js:11)
    at o (bmfont.js:1)
    at r (bmfont.js:1)
    at bmfont.js:1
1../lib/utils @ bmfont.js:11
o @ bmfont.js:1
r @ bmfont.js:1
(anonymous) @ bmfont.js:1
GLTFLoader.js:9 Uncaught ReferenceError: THREE is not defined
    at GLTFLoader.js:9

Did it load for you?

[avaer]

If OrbitControls.js is an ES module then it cannot be loaded with a <script src="OrbitControls.js">.

The proper way to do that is probably to use an import directive.

[yahengsu]

Got it, will take a further look! Thanks for the heads up :)

[yahengsu]

Hey @avaer, I added some fixes for those module imports by adding type="module" to those script tags.

However, when I run locally, I still get these errors:

bmfont.js:11 Uncaught ReferenceError: THREE is not defined
    at Object.1../lib/utils (bmfont.js:11)
    at o (bmfont.js:1)
    at r (bmfont.js:1)
    at bmfont.js:1
1../lib/utils @ bmfont.js:11
o @ bmfont.js:1
r @ bmfont.js:1
(anonymous) @ bmfont.js:1
GLTFLoader.js:9 Uncaught ReferenceError: THREE is not defined
    at GLTFLoader.js:9

The bmfont.js error has to due with how THREE is not defined in the file anywhere. I tried to fix this by adding a line

var THREE = require('./three.js')

but this doesn't work since three.js is a local file and not a module. I think it also has to do with how things are exported in three.js (export default), but I'm not sure how you want to approach it.

For the GLTFLoader.js error, it seems to be getting the GLTFLoader.js file from model-loader.exokit.org instead of locally according to Chrome, so I'm a bit stuck on how to solve that error.

If you know any simple fixes for these errors let me know and I'll get right on them! Thanks :)

[avaer]

For THREE.js on the frontend we usually do

import * as THREE from './three.module.js;

https://github.com/webaverse/xrpackage/blob/3a014baeb6526221289535967b8cba8fc9a6d5b4/run.html#L64

Note that we probably shouldn't set it as a window global in this avatars library.

[avaer]

For the GLTFLoader.js error, it seems to be getting the GLTFLoader.js file from model-loader.exokit.org instead of locally according to Chrome

Either we should pull the GLTFLoader from the models-loader page, or not use models-loader and rewrite support into this library. The former is far easier.

[yahengsu]

We were able to fix the issue by changing imports to use three.module.js when possible, and reverting the old three.js file back to its original state. Testing on local and this seems to have fixed the issue, please let us know if this fixes it for you as well!

[avaer]

and reverting the old three.js file back to its original state

This worries me a little bit. The purpose of the PR is to make sure that the working avatars code from xrpackage.org is pushed here correct? So ideally we are not deviating in any version of anything.

[yahengsu]

Yeah I definitely agree, but I think the main issue is that the three.js file was changed to export default THREE; from exports.xxx = yyy; which is causing the import errors specifically in the avatars repo. We figured a solution to this would be to use the old three.js file for the files that had import errors, and to use the new three.modules.js file for files that had no trouble importing them.

For example, the bmfont.js file is importing with var inherits = require('inherits') statements, the files in vrarmik/ and avatar.js are using import THREE from './three.module.js (which is consistent with XRPackage), and other files like Reflector.js and OrbitControls.js are using THREE.Geometry ,THREE.Vector3(), etc. with no import statements at all. The non-uniform import types are probably why some files have trouble importing the new three.js file, which is why we added the old three.js file back for compatibility reasons. Maybe there's some way to overhaul the avatar files to all use a consistent import type?

Really any solutions you have are greatly appreciated! I'm sure there's probably a more correct way to fix things, but I'm not exactly sure what that might be. 😅

[avaer]

This repo has everything already patched though right? https://github.com/webaverse/xrpackage/tree/master/xrpackage/avatars

The point is that this was already overhauled over there. We just need to move those changes into this repo.

[yahengsu]

I think this commit should be good, added window.THREE = THREE; in index.html to fix the last import error with GLTFLoader.js. I also changed some renderer.vr statements to renderer.xr in Reflector.js and index.html to get rid of some three.js warnings. Also changed some references of .applyMatrix() to .applyMatrix4() to clear up some more three.js warnings.

[avaer]

This looks good, it just needs more thorough testing to go through all of the avatars to make sure they are working correctly in WebXR.

[avaer]

@yahengsu Does this mean we can remove the [WIP] tag?

[yahengsu]

Oh yeah, absolutely haha