Closed orlof closed 10 years ago
btw - Thanks, this is a great project!
Thanks for the vote of confidence. Glad you like the project.
I see your point of breaking the old API. However, it's generally bad API design to have a method both return a value as well as modify one of the parameters in place. It can lead to side-effects and undesired behavior if not used correctly. A much better approach is to have strictly one or the other.
Also I would point out that calling Array arr = e.getComponents(new Array())
is allocating/deallocating memory unnecessarily every time. (especially if the entity has a lot of components and the array needs to grow as well) This triggers garbage collection and thus can cause hits to gaming performance.
A better approach might be to have an Array as a class member variable that you pass into the getComponents() call. As long as you are not working in a multithreaded environment, it should be relatively safe to do so. (the array needs to be cleared before every call though) This is much more memory efficient approach.
Alternatively, I can try and think of a way of keeping a similar array as a member of Entity or the ComponentManager. The latter might be better. And have a function:
Array c = entity.getComponents()
But I don't think the original Artemis implementation is a good solution. There are a lot of things that the original designers did well, but there are quite a few that they did poorly. I'm trying to slowly correct that. And unfortunately, that means making backwards incompatible changes at times.
Would love to hear your thoughts.
Also, I'm curious what your use case here is. I generally use component mappers to access the components instead.
I agree that this is generally a bad design, but in this case I don't like the fact that the framework forces me to do this in more effective way.
Here are my thoughts:
I would love a design that allows getComponents() to be used in variety of ways
My reasoning has contradictions, but that is because this method has different use cases. I use it to copy some of the server entities to client when client joins the game. It is not performance critical, so I would like to keep the code as short and simple as possible, without Array -reset/reuse functionality.
Fair enough. I can make a change to return the Array. But it will be of reusable variety not a new instance created every time. I don't want developers not knowingly creating memory leaks.
On a second thought. Seems like a clone method might be more appropriate and efficient here? Or do you need to make modifications to the components during the copy?
On a second thought, clone would be difficult to implement since some components will probably require a deep copy, and I don't want to force a Cloneable interface on the Components.
It seems that the problem described in the title may be a design decision, but I am used to the pattern of "Array arr = e.getComponents(new Array())" and the change breaks the original Artemis API unnecessarily.