ssatguru / BabylonJS-CharacterController

A CharacterController for BabylonJS
Apache License 2.0
217 stars 46 forks source link

Replace the onKeyDown & onKeyUp events #54

Closed dad72 closed 1 year ago

dad72 commented 2 years ago

Replace the onKeyDown and onKeyUp events with a single BABYLON observable that I name _onKeyBoardObservable. So it will always be compatible with future versions of browsers.

dad72 commented 2 years ago

It has been tested and it works. Besides, I also tested with a glTF character and it works much better in terms of collisions with the ground.

ssatguru commented 1 year ago

Sorry for the delay in looking into this but I am not sure why we should complicate things with observable. Why add another library dependency?. The current code is simple, will not become deprecated anytime soon and do not require any additional library

dad72 commented 1 year ago

There are no additional libraries.

Observables are built into Babylon and its more modern use of events.

ssatguru commented 1 year ago

Yes it is in Babylon but is still additional library. It is not part of standard JavaScript. Browser don't support it natively

On Wed, Nov 16, 2022, 11:29 AM David @.***> wrote:

There are no additional libraries.

Observables are built into Babylon and its more modern use of events.

— Reply to this email directly, view it on GitHub https://github.com/ssatguru/BabylonJS-CharacterController/pull/54#issuecomment-1317391957, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVRJDIZZAETBM7SRUXH7VLWIUKXPANCNFSM54AOJMSA . You are receiving this because you commented.Message ID: @.***>

dad72 commented 1 year ago

The CharacterControler refers to Babylon. It can only work with Babylon. Your whole class is not just from Javascript, but from Babylon. So no additional library anyway because your class will not work without the Babylon library.

You are using BABYLON.Vector3... There is nothing more different or additional than using a BABYLON.Observable. Observables are the future, more modern than of simples calls onKeyDown onKeyUp and all the code around it.

But you do as you wish. But I think it's a good improvement to use observables in the future. Javascript is starting to integrate them too, because they are more efficient.

ssatguru commented 1 year ago

I looked into this more. Babylonjs does not use any library to provide observable. They have implemented this pattern themselves. You can see this here https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Misc/observable.ts#L99 Though I am not very keen on adding this, I will research this further to see if it provides any additional value.

dad72 commented 1 year ago

Yes that's what I said Babylonjs does not use any libraries to provide observables. They have implemented this pattern themselves. Try yes to learn about observables.

Observable JavaScript represents a progressive way of handling events, async the activity, and multiple values in JavaScript. These observables are just the functions that throw values and Objects known as observers subscribe to such values that define the callback functions such as error(), next() and complete().