-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Allow components ordering #6648
Conversation
There is a potential (future?) conflict with a script component, which I noticed have the following line: engine/src/framework/components/script/component.js Lines 228 to 232 in 55a3ff8
However, I am not sure what that is. |
I'm just wondering if this is too much. This allows sorting per component instance, with the order being stored for each instance of a component. I suspect that in reality, we need sorting based on the component type, and so we can avoid storing this per component instance, and store it as a static member of the component type. Additionally, I think currently the array gets sorted each time a component is added. It's likely not a problem, as typically we only have a small number of components per entity. And each entity stores the sorted array as well, which is not ideal. We could have a getter to get sorted components, and sort them at that point, as this is called at low frequency, and would avoid storing the array. Thoughts? Just trying to make this as lightweight as possible. |
Agreed, good points. |
Having an order as a static is a better option. Thanks! I tried to remove the sorted array store and move it to a getter, but not sure it is better than before. Now it gets sorted each time when |
How about this variant? |
…to component-order
This feels right to me. I added some small comments. |
Will it be more efficient to insert a component into its correct position at the moment of adding it, and avoid sorting altogether? |
The component are not stored in an array, but an object. And we're trying to avoid storing additional array per entity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last tiny rename suggestion, but otherwise awesome, thanks!
Fixes #6647
The PR allows components to specify in which order they get enabled when multiple components are present on an Entity.
I confirm I have read the contributing guidelines and signed the Contributor License Agreement.