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

New keyboard controls #101

Merged
merged 5 commits into from Dec 28, 2020
Merged

New keyboard controls #101

merged 5 commits into from Dec 28, 2020

Conversation

extremeheat
Copy link
Member

OrbitalControls with a bit of changes:

  • y axis can now be translated with space and shift
  • controls now stored and applied on render instead of keydown
  • event handlers are registered all at once, and can be unregistered
  • supports both WASD/ZQSD by default, but can be remapped by updating MapControls.controlMap by the user

Everything is exposed, so it should be easy to update constants (like the moving speed, keyPanSpeed, verticalTranslationSpeed) from the default settings.

@rom1504
Copy link
Member

rom1504 commented Dec 28, 2020

Thanks for contributing this, I think it'll be pretty useful to have this feature.

seems like the tests are failing

I'm looking at the code, and this seems completely independent from everything.
What does it do exactly? do you think it makes sense to bundle this into prismarine-viewer or maybe we should have a prismarine-controls ? (or a more general voxel-controls ? how general is this?)
Do you think this code will change often? Do you think people might want to tune this?

@extremeheat
Copy link
Member Author

extremeheat commented Dec 28, 2020

Yeah, it's pretty generic, it can be used for any 3d three.js scene.
Looks like the the tests are failing because webpack/babel doesn't support arrow class function synax (these are used to not bind this in event handler callbacks). Looks like it requires a separate babel plugin standard/standard#1165. The code runs fine in node, but should be able to workaround with .bind(this)

It works by taking in a threejs camera, and a dom element, and listens to page events like keyboard, mouse, touch. Then it applies transformations to the threejs camera based on the inputs. For the use case of panning around the map, I don't think there's much need to make changes to the control code except for constants like movement speed or the camera target (the reference point for rotations). I think the default movement constants are a bit slow for example, but it's just preference based.

For things like first person controls or more cinematic camera controls, the user would probably want to define their own controls. If we add in new first-person controls that maps to prismarine-physics instead of the three camera, it would be more integrated to the minecraft/the viewer, but as it stands this should be usable for any threejs program.

}

unregisterHandlers() {
this.element.removeEventListener('pointermove', this.onPointerMove, false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mentioned on discord that registration is broken, and this seems to be the likely case. These calls aren't actually going to remove anything, because this.onPointerMove.bind(this) !== this.onPointerMove. Your best bet might be to autobind class properties in the constructor so that you can use this.onPointerMove everywhere and have it bound to the class.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed.

@extremeheat
Copy link
Member Author

extremeheat commented Dec 28, 2020

Added some dampening/inertia to the electron example so movements are a bit more fluid. Rotations are still a bit annoying, since the rotations happen around the entire scene, maybe they should happen around the center of the screen or what the user is looking at?

Also most of the editable properties are documented at https://threejs.org/docs/#examples/en/controls/OrbitControls, we could add this info to the readme. Or we could take this out and put it to its own repo, but I'm not sure if it's worth it for a single file.

@rom1504
Copy link
Member

rom1504 commented Dec 28, 2020

Can you add doc for what is exposed in the readme ? (Next to the rest of the API)

@rom1504 rom1504 merged commit 6ee6e3d into PrismarineJS:master Dec 28, 2020
@extremeheat extremeheat deleted the controls branch December 30, 2020 04:08
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

Successfully merging this pull request may close these issues.

None yet

3 participants