Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update README and examples #395

Closed
3 of 6 tasks
vincentfretin opened this issue Dec 7, 2022 · 17 comments
Closed
3 of 6 tasks

Update README and examples #395

vincentfretin opened this issue Dec 7, 2022 · 17 comments

Comments

@vincentfretin
Copy link
Member

vincentfretin commented Dec 7, 2022

  • update examples to use aframe 1.3.0
  • add the castle example in the index page
  • Update README, how to use correct gh jsdelivr urls
  • hexgrid example: not listed in examples page, broken, issue with BufferGeometry
  • platforms example: jump not working correctly, it just does a small 10cm jump, not enough to go on top of a cube
  • playground example broken: Uncaught TypeError: ray._updateDirection is not a function at i.raycastToGround (kinematic-body.js:197:9) I think we need to remove kinematic-body and replace by ammo-body="type: kinematic;"?
@vincentfretin
Copy link
Member Author

In the README, update to use the correct urls. For a gh build for example it's
https://cdn.jsdelivr.net/gh/n5ro/aframe-extras@bb7007c/dist/aframe-extras.min.js

@vincentfretin
Copy link
Member Author

vincentfretin commented Dec 7, 2022

Some components I didn't test at all. I don't know in which state they're in:

  • jump-ability
  • mesh-smooth
  • normal-material
  • a-grid
  • a-hex-grid
  • a-tube
  • cube-env-map (Note: cubemaps are now supported natively in A-Frame via the a-cubemap element)

Components that should be removed from the repo:

  • kinematic-body

@vincentfretin
Copy link
Member Author

If some of the above components needs fixing about BufferGeometry, see an update example for ocean component #387
PR welcome if you use those components and you're able to fix them.

@diarmidmackenzie
Copy link
Member

See #407 - I don't think we should attempt to patch up hex-grid at this point, and shouldn't block release 6.2.0 on having a working hex-grid.

A maintained hex-grid component for A-Frame would be nice to have, but given there is no maintained hex grid implementation available at the THREE.js level, it's a big stretch to take that on, and I don't see it as a priority at this time.

I've not seen anyone in the community asking / complaining about the state of hex-grid.

@vincentfretin
Copy link
Member Author

Agree, let's remove the a-hex-grid component from the repository, and also kinematic-body and cube-env-map as already said above.
I think we can remove mesh-smooth as well, it uses geometry.computeVertexNormals() that doesn't exist anymore. and there are better alternatives https://svox.glitch.me and the fork https://github.com/webspace-sdk/smoothvoxels (see https://gfodor.medium.com/rebooting-the-web-in-3d-with-webspaces-9e58847e042c)

Remaining are jump-ability, normal-material, a-grid, a-tube.
normal-material, a-grid are used in some examples and seems to work.
jump-ability is used in the platforms example and is not behaving properly.
a-tube does it still work? There is no live example.

@diarmidmackenzie
Copy link
Member

I think jump-ability needs to be retired alongside kinematic-body.

It's only used in examples alongside kinematic-body, and only makes sense alongside it. Here's why...

Both use the velocity component, which is part of aframe-physics-system but not a part I was aware of until now!
See: https://github.com/c-frame/aframe-physics-system/tree/master/src/components/math

I don't fully understand the velocity component, but it is only used (AFAIK) by kinematic-body and jump-ability. It's not tightly coupled to the physics engine: it doesn't alter the velocity in the physics system, it just externally overlays an additional velocity on top of it. And physics-system doesn't alter the velocity either.

So to give a sensible jumping behaviour, jump-ability needs to be coupled with something like kinematic-body to apply gravity, so that the player doesn't just fly upwards endlessly.

We should also retire the examples that use kinematic-body: platforms, walls & playground. Even if aspects of them work, they are illustrating a pattern that's explicitly no longer recommended.

It would be nice to replace them with examples showing the recommended solution. Unfortunately I don't think we can do so fully...

The recommendation is to use a nav-mesh to constrain player movement. That works for some parts of the problem, but it's not a complete solution, as it doesn't take care of stopping players running into moving entities (e.g. moving platforms), and I'm not sure how it deals with e.g. head-height issues either...

Unity (for example) offers something called a CharacterController, which is part-physics-body, part nav-mesh-agent and handles all the complex interactions between player controls, nav-mesh & physics. PhysX offers one of these, and it might be an interesting project to bring this to A-Frame as part of c-frame/physx. However VR requirements on a character controller are probably different again from a classic desktop FPS... So it's definitely a complicated topic...

It looks like we do already have a basic nav-mesh example here:, so I think that's enough for now.

In summary - let's also remove all of:

  • jump-ability
  • platforms example
  • playground example
  • walls example

That just leaves a-tube. I'll take a look at that.

@diarmidmackenzie
Copy link
Member

a-tube works fine. I created a simple example here, which I can roll into a PR.
https://github.com/diarmidmackenzie/aframe-extras/tree/tubes-example/examples/tubes

@diarmidmackenzie
Copy link
Member

normal-material doesn't work AFAICT - only used in the castle example, and applied to an invisible mesh.
I tried setting normal-material on my tubes example, and just got a white material - looking into that now.

@diarmidmackenzie
Copy link
Member

diarmidmackenzie commented Jan 9, 2023

normal-material was partly broken - worked with GLTFs where model loads in asynchronously. It didn't work with simple geometries like a-sphere and a-tube where the geometry is set up synchronously.

I have a 1-line fix for this which I'll put in a PR as well.

Tubes example now covers normal-material as well...
image

@diarmidmackenzie
Copy link
Member

diarmidmackenzie commented Jan 10, 2023

PRs #408, #409 and #410 should fix most of the remaining issues here.

What remains? retiring cube-env-map & mesh-smooth - will do one more PR for those.

Given that we are removing components (hence changing the interface), according to SemVer we should move to 7.0.0 rather than 6.2.0?

I didn't apply that change (awaiting discussion), but that would be my recommendation.

@diarmidmackenzie
Copy link
Member

I think we can remove mesh-smooth as well, it uses geometry.computeVertexNormals() that doesn't exist anymore. and there are better alternatives https://svox.glitch.me/ and the fork https://github.com/webspace-sdk/smoothvoxels (see https://gfodor.medium.com/rebooting-the-web-in-3d-with-webspaces-9e58847e042c)

I'm not sure about this.

computeVertexNormals does still exist in THREE.js (now on BufferGeometry)

Smooth Voxels is very cool, but it's not a drop-in replacement, as it can't take a generic Mesh as an input.

I don't know how widely used this is, but I don't see a reason to remove. I suggest we keep this & create an example to illustrate it's use. I'll do a PR for that.

@diarmidmackenzie
Copy link
Member

#409 extended to also remove a-cubemap (rolled into that PR to reduce merge conflicts)

@vincentfretin
Copy link
Member Author

Ok for 7.0.0, you can update #409 to reference 7.0.0 and rebase now that #408 is merged.

@vincentfretin
Copy link
Member Author

You're right about computeVertexNormals and mesh-smooth.

@diarmidmackenzie
Copy link
Member

You're right about computeVertexNormals and mesh-smooth.

Actually I think I was wrong :-)

@diarmidmackenzie
Copy link
Member

@vincentfretin

Based on your original list here, I believe this can now be closed?

(the three examples still to fix have all been removed...)

@vincentfretin
Copy link
Member Author

indeed, all good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants