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

Implement Extend for World #96

Closed
wants to merge 1 commit into from
Closed

Implement Extend for World #96

wants to merge 1 commit into from

Conversation

Anders429
Copy link
Owner

This fixes #14.

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2022

Codecov Report

Merging #96 (f0dbf12) into dev (cb43f5d) will decrease coverage by 0.34%.
The diff coverage is 89.76%.

@@            Coverage Diff             @@
##              dev      #96      +/-   ##
==========================================
- Coverage   95.15%   94.81%   -0.35%     
==========================================
  Files          57       61       +4     
  Lines        9428     9542     +114     
==========================================
+ Hits         8971     9047      +76     
- Misses        457      495      +38     
Impacted Files Coverage Δ
src/query/view/seal.rs 100.00% <ø> (ø)
src/world/mod.rs 99.52% <57.14%> (-0.48%) ⬇️
src/entities/iter.rs 78.78% <78.78%> (ø)
src/entities/mod.rs 96.87% <100.00%> (-3.13%) ⬇️
src/entities/raw.rs 100.00% <100.00%> (ø)
src/entities/seal/into_raw.rs 100.00% <100.00%> (ø)
src/entities/seal/length.rs 90.62% <100.00%> (ø)
src/entity/seal/unzip.rs 100.00% <100.00%> (ø)
src/entities/seal/storage.rs 38.46% <0.00%> (-36.93%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Anders429
Copy link
Owner Author

This is currently in development. While this solution works, it has some significant performance penalties for inserting into a World's Archetype that doesn't have any values already in it. It would be great if there were some way the specialized FromIterator<vec::IntoIter> implementation for Vec could be utilized, but that might not actually be possible.

I'm hesitant to merge this if it hurts performance enough. I'm currently seeing about a 3x increase in performance, which is significant (although it does put the performance right there next to the other ECS libraries, instead of way out in front of them like it currently is). While extending from an iterator is something that would be nice to have, I'm not sure that it is strictly necessary for the large amount of use cases. Most people will be just inserting entities directly.

@Anders429 Anders429 linked an issue Aug 24, 2022 that may be closed by this pull request
@Anders429
Copy link
Owner Author

Note: here was the proof of concept for the Unzip heterogeneous list trait I added: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c9497548e94e343ac177ac3148ac991f

@Anders429 Anders429 added C - Enhancement Category: New feature or request. S - Needs Investigation Status: Further investigation is needed. P - Low Priority: Not particularly urgent. A - Storage Area: Storage inside a World. V - Minor Breaking Change Versioning: Requires a minor bump according to semver. labels Sep 15, 2022
@Anders429
Copy link
Owner Author

As it currently stands, this PR is not going to be merged. It has grown stale over the last several months, and it is not the correct solution to the problem.

This feature may not actually be possible until specialization is stabilized.

@Anders429 Anders429 closed this Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A - Storage Area: Storage inside a World. C - Enhancement Category: New feature or request. P - Low Priority: Not particularly urgent. S - Needs Investigation Status: Further investigation is needed. V - Minor Breaking Change Versioning: Requires a minor bump according to semver.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Extend for World
2 participants