-
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
Scrollview remove entity reference #7206
Scrollview remove entity reference #7206
Conversation
Co-authored-by: Will Eastcott <willeastcott@gmail.com>
… into renderRootBoneOpt
@Maksims - could you please merge main to it and resolve the conflict |
Done. |
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.
This looks good, but I wonder if we could avoid some code duplication here, which adds some complexity. It could do with some wrapper of some of the code?
I thought of some more unified way to handle entity setting, subscribe/unsubscribe mechanics also, but every case is so slightly different. This PR reduces overall amount of code in the Engine. I guess some refactoring is possible as a separate PR in the future, but that would require to refactor whole component's code. |
* optimize render component rootBone property * expose rootBone property * Update src/framework/components/render/component.js Co-authored-by: Will Eastcott <willeastcott@gmail.com> * use jsdo import * . * ButtonComponent remove EntityReference * remove reliance on component system global event lists * remove EntityReference from ScrollbarComponent * remove EntityReference, optimize ScrollVeiw and Scrollbar * lint --------- Co-authored-by: Will Eastcott <willeastcott@gmail.com> # Conflicts: # src/framework/components/scroll-view/component.js # src/framework/components/scroll-view/data.js # src/framework/components/scrollbar/component.js # src/framework/components/scrollbar/data.js # test/framework/utils/entity-reference.test.mjs
merged to v2 |
This PR depends on #7194 and #7204 (and includes them already), merge them first please so diff is more concise.
This PR does not change API or behavior.
As mentioned in #7195 - EntityRefence need to go. This PR completes that, by removing a last dependency on it and also removing reliance on ComponentSystem's global event lists, which leads to exponentially higher costs of creating/removing related components (Element especially).
There is a significant performance improvement when creating/removing such components similar to other two PR's mentioned above. This also benefits ElementComponent's creation/removal performance, as every ScrollView and Scrollbar components were subscribing to ElementComponentSystem's global lists.
I confirm I have read the contributing guidelines and signed the Contributor License Agreement.