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.

Sign up for GitHub

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

Jump to bottom

StorageType of resources should not be configurable #3361

Closed
alice-i-cecile opened this issue Dec 17, 2021 · 1 comment
Closed

StorageType of resources should not be configurable #3361

alice-i-cecile opened this issue Dec 17, 2021 · 1 comment
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change

Comments

@alice-i-cecile
Copy link
Member

The StorageType of resources is configurable in the ComponentDescriptor impl.

bevy/crates/bevy_ecs/src/component.rs

Line 180 in 7356f15

impl ComponentDescriptor {

This doesn't exactly make sense, as we only ever have one of each resource, and it's not immediately clear to me that this is meaningful.

In practice, this is always called with StorageType::default() as its argument (defaulting to TableStorage). This should be moved to within these methods, and the argument should be removed.

    pub fn init_resource<T: Resource>(&mut self) -> ComponentId {
        // SAFE: The [`ComponentDescriptor`] matches the [`TypeId`]
        unsafe {
            self.get_or_insert_resource_with(TypeId::of::<T>(), || {
                ComponentDescriptor::new_resource::<T>(StorageType::default())
            })
        }
    }
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change labels Dec 17, 2021
@alice-i-cecile
Copy link
Member Author

SparseStorage may actually be a more reasonable choice, but eh.

@r4gus r4gus mentioned this issue Dec 30, 2021
Closed
@bors bors bot closed this as completed in c641a90 Dec 30, 2021
mockersf pushed a commit to mockersf/bevy that referenced this issue Jan 1, 2022
@r4gus @mockersf
StorageType parameter removed from ComponentDescriptor::new_resource (b…
a441178 
…evyengine#3495)

# Objective

Remove the `StorageType` parameter from `ComponentDescriptor::new_resource` as discussed in bevyengine#3361.

- fixes bevyengine#3361

## Solution

- Parameter removed.
- Basic docs added.

## Note

Left a [comment](bevyengine#3361 (comment)) about `SparseStorage` being the more reasonable choice.



Co-authored-by: r4gus <david@thesugar.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Merged by Bors] - StorageType parameter removed from ComponentDescriptor::new_resource
1 participant
@alice-i-cecile