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

Why are component props put behind getter/setters? #16

Closed
fenomas opened this issue Jul 22, 2015 · 2 comments
Closed

Why are component props put behind getter/setters? #16

fenomas opened this issue Jul 22, 2015 · 2 comments

Comments

@fenomas
Copy link
Contributor

fenomas commented Jul 22, 2015

Hi -- This library looks really nice and well-designed, I'm trying it out in a game!

One question: why are component properties always wrapped in getter/setter functions, even though the manager isn't an event emitter?

If the intent is that the user can optionally add a manager.emit function, wouldn't it make sense to make getter/setter wrapping optional to avoid overhead in the default case?

E.g.:

EntityManager.prototype.addComponentsToEntity = function (
              componentIds, 
              entityId, 
              wrapWithGetterSetters=false) {

or the like?

Thanks!

@adngdb
Copy link
Owner

adngdb commented Jul 22, 2015

Hi! Thanks for using it, I'm looking forward to seeing what you will do with it.

The intent you state is correct, and you are right that it is not optimal. However, on the solution side, I think we can simply check if manager.emit is a function before making getters/setters, and just not make them if it isn't. That means adding an emit function to a manager after declaring some components won't emit anything, but I don't expect that people will do do those sorts of things, and we can always say it's a feature... :)

What do you think?

@fenomas
Copy link
Contributor Author

fenomas commented Jul 22, 2015

Hey,

I was actually going to suggest that 😃 But I wasn't sure if it matched your intent.
I think it would be a great fix!

@adngdb adngdb closed this as completed in d920aba Jul 23, 2015
adngdb added a commit that referenced this issue Jul 23, 2015
…-emit

Fixes #16 - Do not add getters and setters when there is no emit method.
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