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

Add support for typing in EventDispatcher #17134

Closed
amiorenstx opened this issue Jul 31, 2019 · 5 comments
Closed

Add support for typing in EventDispatcher #17134

amiorenstx opened this issue Jul 31, 2019 · 5 comments

Comments

@amiorenstx
Copy link
Contributor

Or take it a step further:
I would consider migrating the whole library to typescript and adding the rxjs library.
With rxjs you will be able to subscribe to different changes such as position changes etc
Also you will be able to know which type of messages are emitted from different events and be able to thorttle messages and buffer them

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 31, 2019

As mentioned here #15545 three.js will not be converted to TS. The library provides type declaration files for the core and example JS code which should be sufficient for TS users.

Also you will be able to know which type of messages are emitted from different events and be able to thorttle messages and buffer them

You can implement the same logic with JS, just differently (e.g. put the message type information in the event object). However, I don't think this stuff is necessary in the engine.

@Usnul
Copy link
Contributor

Usnul commented Aug 1, 2019

Ha, interesting discussion. At some point three.js started to create actual objects and send them as events, right now they are of form:

{
type:"add"
}

but the reason why I know is, I re-populate my scene every frame, because of custom culling mechanism, and I found, via profiling, that three.js was generating a decent amount of 🗑 garbage, so I thought to myself 🤔

"hey, three.js is usually super good at keeping GC to a minimum, what gives?"

turns out - new event objects. Now I bypass scene.add and manipulate children property directly.

My point is - I would prefer not to see more of an event framework in three.js, it's just about too heavy from my perspective as is. 🏋️‍♀️

@gkjohnson
Copy link
Collaborator

"hey, three.js is usually super good at keeping GC to a minimum, what gives?"

turns out - new event objects.

This is interesting -- I hadn't considered that. Classes like OrbitControls create objects once and reuse them to dispatch events. It seems like that would be a reasonable improvement to the add and remove events in Object3D?

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 1, 2019

It seems like that would be a reasonable improvement to the add and remove events in Object3D?

I think so, too. I'm not sure if there was a specific reason to create new objects. Reusing event objects for both methods seems to make more sense.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 2, 2019

Anyway, we've got sidetracked. The issue itself can be closed.

The idea of reusing event objects is important and should be discussed in another topic (it has nothing to do with TS).

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

4 participants