Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

When adding a new component return it instead of the entity itself #24

Closed
fernandojsg opened this issue Jul 3, 2019 · 4 comments
Closed

Comments

@fernandojsg
Copy link
Member

fernandojsg commented Jul 3, 2019

Currently addComponent return the entity instead of the component, so you could chain them as:

entity.addComponent(ComponentA).addComponent(ComponentB);

But if you want to set component values directly without passing an object on the addComponent function, it could be nice to return the component directly:

let componentA = entity.addComponent(ComponentA);
componentA.value = 23;

One question would be if we would like that behaviour to be the same as if we would do getMutableComponent so it will trigger the changed. But as we are just creating it, I believe that just triggering the added event should be enough for now.

@robertlong
Copy link
Member

Yes, I'd really like addComponent to return the component directly.

I'm not sure what should happen with regard to triggering the changed event. I think I would just handle any logic that deals with the properties in the added event in the same way that I would handle them if I did this addComponent(ComponentA, { some: "props" })

@netpro2k
Copy link
Collaborator

netpro2k commented Jul 3, 2019

Mostly indifferent to returning the component or entity, but it doesn't seem right to be modifying a component ref you didn't get from getMutableComponent so I maybe prefer returning the entity to avoid that confusion. I think if we end up moving pooling out of addComponent you end up passing a component instance into addComponent and so this becomes unnecessary anyway.

@robertlong
Copy link
Member

Yeah, just passing the instance is probably fine too. It just depends on how we implement pooling. If addComponent(ComponentA) creates a new instance of ComponentA via its ComponentManager then I'd want addComponent() to return the instance.

If you acquire a new component instance from some other method like world.createComponent(ComponentA) or world.getComponentManager(ComponentA).create() then that would be fine as well. I think this is closely related to how we handle component pooling so maybe we should figure that out first?

@fernandojsg
Copy link
Member Author

fernandojsg commented Sep 17, 2019

Leaving it like this (= returning the entity) in favor to https://github.com/fernandojsg/ecsy/issues/81 to avoid side effects or unwanted use cases as storing a reference to an component or so

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

No branches or pull requests

3 participants