Street geo

[Algorush]

No description provided.

[netlify]

✅ Deploy Preview for 3dstreet-core-builds ready!

Name	Link
🔨 Latest commit	6501ff3
🔍 Latest deploy log	https://app.netlify.com/sites/3dstreet-core-builds/deploys/664cc851b1ce8500086317b4
😎 Deploy Preview	https://deploy-preview-516--3dstreet-core-builds.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[Algorush]

I have changed this example for testing: /examples/google-tiles/
Now you can change the component parameters like this in console:

r = document.getElementById('reference-layers')
r.setAttribute('street-geo', 'maps', 'mapbox2d')
r.setAttribute('street-geo', 'maps', 'google3d')
r.setAttribute('street-geo', 'maps', 'google3d, mapbox2d')
r.setAttribute('street-geo', 'maps', '')

The lat/long change still needs to be tested

[kfarr]

mapTypes could be defined in init not update

[Algorush]

I renamed the map creation functions. Now the function has a name: MapType + "Create". Similar to the update functions: MapType + "Update".
I don't know, maybe it's not good practice to do like this?

[kfarr]

Elevation is the requested elevation of the scene's origin 0 0 0. In other words, if a user has lat 30, long 30, elevation 30, then I assume the elevation is 30 meters above sea level.

But google 3d tiles takes height which is a different value. Here is a sample of how I converted elevation into height in a glitch prototype project:

          // Default is SF Fort Mason Northwest Pier Streetlight
          const demoLat = queryParams.get("lat") ?? 37.807193;
          const demoLong = queryParams.get("long") ?? -122.431964;
          const elevationHeightConstant = 32.49158 // a "height" of -26 = 6.49m elevation therefore 32.49158
          const demoHeight = queryParams.get("elevation") - elevationHeightConstant ?? -26
          console.log(demoHeight);
          console.log(demoLat, demoLong);
          this.el.setAttribute('loader-3dtiles', 'long', demoLong)
          this.el.setAttribute('loader-3dtiles', 'lat', demoLat)
          this.el.setAttribute('loader-3dtiles', 'height', demoHeight)

[vincentfretin]

You have a mix of spaces and tabs in the file, be sure to replace all tabs by spaces.

[vincentfretin]

This doesn't seem to be used.

[kfarr]

Next:

• documentation for the component
• test - does it create a child with the proper component properties? does it update the child when the wrapper properties are updated? (coordinates or map type)

[Algorush]

added documentation and test based on chai lib. Test could be run from /test/browserTests/street-geo-test.html
Chai library supports execution through the browser, so it is more convenient to use for testing when working with DOM, AFRAME and Three.js elements.

[Algorush]

I also added a .node.js ending to all existing tests, so that only tests with a .node.js ending would be run in test command through package.json.
For now, browser tests need to be run manually. Perhaps they can also be configured to launch via gitHub CI.
... yes, chatGPT suggests that browser tests can be run in Github Actions via puppeter or playwright

[vincentfretin]

Do you really need to wait 1s? We don't have much tests so that may be fine but if we can avoid that it would be better. You probably just need two setTimeout to be sure the street-geo is initialized and the child entity with components are created and initialized.

setTimeout(() => {
   setTimeout(() => {
     const mapbox2dElement = el.querySelector('[data-layer-name="Mapbox Satellite Streets"]');
      const google3dElement = el.querySelector('[data-layer-name="Google 3D Tiles"]');
      expect(mapbox2dElement).to.exist;
      expect(google3dElement).to.exist;
      done();
  });
});

[Algorush]

Thanks for the tip!

[vincentfretin]

For tests, the main convention I saw in several projects is to mirror the directory structure and end the test filename with .spec.js or .test.js. The aframe repository for example uses .test.js convention.
So like this:

3dstreet/
├── src/
│   ├── components/
│   │   └── street-geo.js
├── test/
│   ├── components/
│   │   └── street-geo.test.js

[vincentfretin]

Those new tests should be able to be run with mocha and jsdom, no need to spawn a browser. I see existing tests were using jsdom-global already.

[Algorush]

Those new tests should be able to be run with mocha and jsdom, no need to spawn a browser. I see existing tests were using jsdom-global already.

Hi, existing tests in 3DStreet do not use AFRAME. I had difficulty to run AFRAME in Node.js environment. And I thought that running AFRAME in a browser environment with the chai library would allow to write tests more conveniently than in Node.js environment. I have now discovered information about using AFRAME in the Node.js environment. I'll try to do it this way

[Algorush]

I tried using only jsdom, mocha. I still can't run tests for the street-geo component that using AFRAME in a Node.js environment. I was using this info in AFRAME docs about running AFRAME in Node.js, but I'm getting errors in the console when running tests:

import * as SUPER_THREE from 'super-three';
^^^^^^

SyntaxError: Cannot use import statement outside a module

that I haven't found a way to fix. I see these options so far:

1. I can make a simplified version of the street-geo component for the test by disabling the parts of the code that access AFRAME. Something like a test mode without AFRAME functions. If we only test adding/removing entities.
2. Use a testing environment similar to the one used when testing AFRAME: https://github.com/aframevr/aframe/tree/master/tests
3. Use the tests in chai browser environment, perhaps it will be easier than option 2: chai + mocha + puppeter for github Actions (it’s not difficult to set up)

Another option is to further try to fix the errors in point 1, that is, change the three.js code referenced by Aframe. I already had this experience a year ago when I adapted Three.js to work on the server side. But I think it would be easier to do 2 or 3

[vincentfretin]

The aframe doc is not be up to date, using require('aframe/src'); only works in a webpack environment like karma-webpack because of aframe/src/lib/three.module.js that's the only code where it uses ES module and also the example in the aframe repo with npm run test:node is currently broken. Now that node 22 supports requiring ES module with require("somemodule.mjs") see https://nodejs.org/en/blog/announcements/v22-release-announce#support-requireing-synchronous-esm-graphs,
I made an attempt to fix that in aframe repo in https://github.com/vincentfretin/aframe/tree/test-node and actually it works, but this can only tests basic functions because node environment lacks customElements, even with jsdom.
So yeah long story short, you still need to spawn a browser via karma for example like aframe tests are using.

[vincentfretin]

So yeah for now executing the tests manually by accessing
http://192.168.1.15:7001/test/browserTests/street-geo-test.html
that is executing test/browserTests/street-geo-test-bdd.mjs is enough. (see if you can use the setTimeout(setTimeout there too.)
and we can remove test/street-geo.test.js and aframe dependency.
We'll look at using karma and karma-webpack in another PR @kfarr I think.

[vincentfretin]

FYI I made this PR aframevr/aframe#5522