aframe-extras icon indicating copy to clipboard operation
aframe-extras copied to clipboard

All components that use three.js Geometry such as <a-ocean> are broken in A-Frame 1.2.0

Open crcdng opened this issue 4 years ago • 9 comments

aframe-extras.primitives.js:82 Uncaught TypeError: geometry.mergeVertices is not a function
    at i.play (aframe-extras.primitives.js:82)
    at i.<anonymous> (aframe-v1.2.0.min.js:2590)
    at HTMLElement.play (aframe-v1.2.0.min.js:2582)
    at HTMLElement.play (aframe-v1.2.0.min.js:2582)
    at HTMLElement.play (aframe-v1.2.0.min.js:2582)
    at HTMLElement.<anonymous> (aframe-v1.2.0.min.js:2596)
    at HTMLElement.emit (aframe-v1.2.0.min.js:2586)
    at aframe-v1.2.0.min.js:2586

Geometryhas been removed in three.js 125 https://github.com/mrdoob/three.js/wiki/Migration-Guide

All components that rely on it have to be re-written using BufferGeometry instead.

crcdng avatar Feb 06 '21 02:02 crcdng

here is a quickly hacked together update for <a-ocean>, each line marked #OLD is to be replaced by the line below it.

AFRAME.registerPrimitive('a-ocean', {
  defaultComponents: {
    ocean: {},
    rotation: { x: -90, y: 0, z: 0 }
  },
  mappings: {
    width: 'ocean.width',
    depth: 'ocean.depth',
    density: 'ocean.density',
    amplitude: 'ocean.amplitude',
    amplitudeVariance: 'ocean.amplitudeVariance',
    speed: 'ocean.speed',
    speedVariance: 'ocean.speedVariance',
    color: 'ocean.color',
    opacity: 'ocean.opacity'
  }
});

AFRAME.registerComponent('ocean', {
  schema: {
    // Dimensions of the ocean area.
    width: { default: 10, min: 0 },
    depth: { default: 10, min: 0 },

    // Density of waves.
    density: { default: 10 },

    // Wave amplitude and variance.
    amplitude: { default: 0.1 },
    amplitudeVariance: { default: 0.3 },

    // Wave speed and variance.
    speed: { default: 1 },
    speedVariance: { default: 2 },

    // Material.
    color: { default: '#7AD2F7', type: 'color' },
    opacity: { default: 0.8 }
  },

  /**
   * Use play() instead of init(), because component mappings – unavailable as dependencies – are
   * not guaranteed to have parsed when this component is initialized.
   */
  play: function () {
    const el = this.el;
    const data = this.data;
    let material = el.components.material;

    let geometry = new THREE.PlaneGeometry(data.width, data.depth, data.density, data.density);
    // geometry.mergeVertices(); #OLD
    geometry = THREE.BufferGeometryUtils.mergeVertices(geometry);
    this.waves = [];
    // for (let v, i = 0, l = geometry.vertices.length; i < l; i++) { #OLD
    for (let v, i = 0, l = geometry.attributes.position.count; i < l; i++) {
      // v = geometry.vertices[i]; #OLD
      v = geometry.attributes.position;
      this.waves.push({
        // z: v.z, #OLD
        z: v.getZ(i),
        ang: Math.random() * Math.PI * 2,
        amp: data.amplitude + Math.random() * data.amplitudeVariance,
        speed: (data.speed + Math.random() * data.speedVariance) / 1000 // radians / frame
      });
    }

    if (!material) {
      material = {};
      material.material = new THREE.MeshPhongMaterial({
        color: data.color,
        transparent: data.opacity < 1,
        opacity: data.opacity,
        flatShading: true
      });
    }

    this.mesh = new THREE.Mesh(geometry, material.material);
    el.setObject3D('mesh', this.mesh);
  },

  remove: function () {
    this.el.removeObject3D('mesh');
  },

  tick: function (t, dt) {
    if (!dt) return;

    // const verts = this.mesh.geometry.vertices; #OLD
    const verts = this.mesh.geometry.attributes.position.array;
    // for (let v, vprops, i = 0; (v = verts[i]); i++) { #OLD
    for (let i = 0, j = 2; i < this.waves.length; i++, j = j + 3) {
      const vprops = this.waves[i];
      // v.z = vprops.z + Math.sin(vprops.ang) * vprops.amp; #OLD
      verts[j] = vprops.z + Math.sin(vprops.ang) * vprops.amp;
      vprops.ang += vprops.speed * dt;
    }
    // this.mesh.geometry.verticesNeedUpdate = true; #OLD
    this.mesh.geometry.attributes.position.needsUpdate = true;
  }
});

crcdng avatar Feb 06 '21 12:02 crcdng

Does this pull request look right? https://github.com/n5ro/aframe-extras/pull/352

n5ro avatar Mar 23 '21 18:03 n5ro

I don't see open PRs, it is currently in branch geometry not in master, right? Sorry I have missed the conversation. I just downloaded both master (commit https://github.com/n5ro/aframe-extras/commit/afcebe83edfe9bcbda0dcf055ff399f34d8f2a5f), and branch geometry (https://github.com/n5ro/aframe-extras/commit/1aad660e69849c51995633a83120e2d31f8ef4b4), included dist/aframe-extras.js and still got the same error in both.

crcdng avatar Apr 10 '21 12:04 crcdng

I opened a PR to revert the code changes that I made to Ocean, so that you can at least see what changes I made, and test them. In hindsight I ought to have followed a more correct process of letting others preview and test the PR further before approving it.

Please review the open PR to revert the changes I made to Ocean to see what was done last time, so you can advise on how to make further improvements.

In addition I am also trying to fix navmesh so that it works in Aframe 1.2.0

Help with navmesh testing, and writing https://glitch.com/edit/#!/navmesh1 https://glitch.com/edit/#!/navmesh https://glitch.com/edit/#!/navmesh2

Feel free to read the commented code in each index file for details. I am trying to replace Three.Geometry which has been deleted from threejs with BufferGeometry. I could use some help testing this and writing this code, please remix the links and try to get it working also. In the meantime use Aframe 1.1.0 with A-Frame Extras until we get this fixed.

n5ro avatar Apr 11 '21 13:04 n5ro

Feel free to read the commented code in each index file for details. I am trying to replace Three.Geometry which has been deleted from threejs with BufferGeometry. I could use some help testing this and writing this code, please remix the links and try to get it working also. In the meantime use Aframe 1.1.0 with A-Frame Extras until we get this fixed.

Hi thanks.

Please have a look at my post above - https://github.com/n5ro/aframe-extras/issues/348#issuecomment-774473193 from Feb 6 - I had already hacked together a version that works for me with A-FRAME 1.20. I commented the edits I made.

However this is not production quality code and unfortunately I prefer to write my own components without the A-FRAME tooling, which prevents me to contribute more to "official" builds.

Happy to help / test though - ideally from a compiled version in /dist.

crcdng avatar Apr 11 '21 15:04 crcdng

Your comment #348 is what I used to create the changes that you can see highlighted in the a-ocean "revert" PR I created to highlight my implementation of your suggestions. If the code is working there is no need to merge the revert PR.

n5ro avatar Apr 11 '21 22:04 n5ro

There is a newly discovered issue with the Aframe Physics System Cannon program, use the Ammo version instead for now. If you want to help fix it, please read and comment on the issue here https://github.com/n5ro/aframe-physics-system/issues/189

n5ro avatar Apr 20 '21 10:04 n5ro

Feel free to read the commented code in each index file for details. I am trying to replace Three.Geometry which has been deleted from threejs with BufferGeometry. I could use some help testing this and writing this code, please remix the links and try to get it working also. In the meantime use Aframe 1.1.0 with A-Frame Extras until we get this fixed.

Hi thanks.

Please have a look at my post above - #348 (comment) from Feb 6 - I had already hacked together a version that works for me with A-FRAME 1.20. I commented the edits I made.

However this is not production quality code and unfortunately I prefer to write my own components without the A-FRAME tooling, which prevents me to contribute more to "official" builds.

Happy to help / test though - ideally from a compiled version in /dist.

i3 I want to ask you for help with your expertise on the differences between three.geometry and three.buffergeometry because obviously there is a lot more to change than just the word Buffer. As you demonstrated with your suggestions above. Quote: " // const verts = this.mesh.geometry.vertices; #OLD const verts = this.mesh.geometry.attributes.position.array; // for (let v, vprops, i = 0; (v = verts[i]); i++) { #OLD for (let i = 0, j = 2; i < this.waves.length; i++, j = j + 3) { const vprops = this.waves[i]; // v.z = vprops.z + Math.sin(vprops.ang) * vprops.amp; #OLD verts[j] = vprops.z + Math.sin(vprops.ang) * vprops.amp; vprops.ang += vprops.speed * dt; } // this.mesh.geometry.verticesNeedUpdate = true; #OLD this.mesh.geometry.attributes.position.needsUpdate = true; }" Well the next change I think is that Don McCurdy is updating three to Cannon https://github.com/donmccurdy/three-to-cannon but there may be other tiny changes that I need to search for in Aframe Extras and Aframe Physics System that refer to the old three.geometry code that I need to update, replace, or figure out what to do. How did you come up with the list of changes that needed to be made? How did you identify what needed to be changed? Surely not from memory? I'm asking sort of what process can I follow to identify everything that needs to be changed, and how to change it correctly. I guess I would have to scour both the threejs Buffergeometry script and compare it to the threejs Geometry script? Do you know of any resource or tool to make this more straight forward?

n5ro avatar May 10 '21 18:05 n5ro

How did you come up with the list of changes that needed to be made? How did you identify what needed to be changed? Surely not from memory? I'm asking sort of what process can I follow to identify everything that needs to be changed, and how to change it correctly. I guess I would have to scour both the threejs Buffergeometry script and compare it to the threejs Geometry script? Do you know of any resource or tool to make this more straight forward?

So actually I moved by trial and error, starting with a minimal scene only containing a <a-ocean> element, going back and forth between error messages, the three.js examples / documentation and stackoverflow until the ocean reappeared. I still don't know if the code is the correct way to do it therefore labelled "hacked together". I hope the three.js folks find a better way to introduce breaking changes like that.

crcdng avatar May 13 '21 15:05 crcdng

Wow there were lots of discussion here :) I fixed the ocean component in #387 For other components, something similar may be needed.

vincentfretin avatar Dec 04 '22 08:12 vincentfretin

I'll close this issue. If there are other components that needs fixing, this will be part of #395

vincentfretin avatar Dec 07 '22 13:12 vincentfretin