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

Fix memory leak in dynamic ECS example #11461

Merged
merged 6 commits into from
Jan 22, 2024

Conversation

james-j-obrien
Copy link
Contributor

@james-j-obrien james-j-obrien commented Jan 21, 2024

Objective

Solution

  • Rather than allocate the memory manually instead store a collection of Vec that will be dropped after it is used.

I must have misinterpreted OwningPtrs semantics when initially writing this example. I believe we should be able to provide better APIs here for inserting dynamic components that don't require the user to wrangle so much unsafety. We have no other examples using insert_by_ids and our only tests show it being used for 1 or 2 values with nested calls to OwningPtr::make despite the function taking an iterator. Rust's type system is quite restrictive here but we could at least let OwningPtr::new take non u8 NonNull.

I also agree with #11459 that we should generally be trying to simplify and clarify this example.

@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Jan 22, 2024
Copy link
Contributor

@nicopap nicopap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mockersf mockersf added C-Examples An addition or correction to our examples A-Pointers Relating to Bevy pointer abstractions labels Jan 22, 2024
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 22, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 22, 2024
Merged via the queue into bevyengine:main with commit f8191be Jan 22, 2024
25 checks passed
@james-j-obrien james-j-obrien deleted the dynamic-example-fix branch January 22, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Pointers Relating to Bevy pointer abstractions C-Examples An addition or correction to our examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants