Skip to content

Conversation

@Trashtalk217
Copy link
Contributor

@Trashtalk217 Trashtalk217 commented Sep 8, 2025

This is part of #19731.

Resources as Components

Motivation

More things should be entities. This simplifies the API, the lower-level implementation and the tools we have for entities and components can be used for other things in the engine. In particular, for resources, it is really handy to have observers, which we currently don't have. See #20821 under 1A, for a more specific use.

Current Work

This removes the resources field from the world storage and instead store the resources on singleton entities. For easy lookup, we add a HashMap<ComponentId, Entity> to World, in order to quickly find the singleton entity where the resource is stored.

Because we store resources on entities, we derive Component alongside Resource, this means that

#[derive(Resource)]
struct Foo;

turns into

#[derive(Resource, Component)]
struct Foo;

This was also done for reflections, meaning that

#[derive(Resource, Reflect)]
#[refect(Resource)]
struct Bar;

becomes

#[derive(Resource, Component, Reflect)]
#[refect(Resource, Component)]
struct Bar;

In order to distinguish resource entities, they are tagged with the IsResource component. Additionally, to ensure that they aren't queried by accident, they are also tagged as being internal entities, which means that they don't show up in queries by default.

Drawbacks

  • Currently you can't have a struct that is both a Resource and a Component, because Resource expands to also implement Component, this means that this throws a compiler error as it's implemented twice.
  • Because every reflected Resource must also implement ReflectComponent you need to import bevy_ecs::reflect::ReflectComponent every time you use #[reflect(Resource)]. This is kind of unintuitive.

Future Work

  • Simplify Access in the ECS, to only deal with components (and not components and resources).
  • Newtype Res<Resource> to Single<Ref<Resource>> (or something similair).
  • Eliminate ReflectResource.
  • Take stabs at simplifying the public facing API.

Copy link
Contributor

@chescock chescock left a comment

Choose a reason for hiding this comment

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

Thanks for all your work pushing this forward! I imagine it's frustrating to be asked to completely rewrite it, and then to rewrite it back the first way again. But having an actual implementation of both approaches really is valuable for evaluating the tradeoffs!

And now, on to the nitpicking...


#[test]
#[should_panic]
fn filtered_resource_mut_conflicts_write_with_resmut() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did these tests get removed? The behavior they're checking seems important to preserve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think deleting the filtered_resource_mut_conflicts_write_with_resmut test was a mistake and I deleted filtered_resource_reflect because I had a hard time understanding the test and since ReflectResource was changed so much I didn't know how to fix it.

I'll look into this again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've restored filtered_resource_mut_conflicts_write_with_resmut, but not filtered_resource_reflect. The test fundamentally doesn't make sense with the new ReflectResource implementation. I'm 90% sure that I'm going to redo FilteredResources at a later date, but that's a separate PR.

if let Some(entity) = self.resource_entities.get(component_id)
&& let Ok(entity_ref) = self.get_entity(*entity)
{
return entity_ref.contains_id(component_id) && entity_ref.contains::<IsResource>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why check for IsResource here? The hooks should prevent duplicates, and the resource_entities map is the authoritative source of whether this was a "resource entity" or not.

... ah, it's because we need that check in Res parameters so that they don't conflict with queries that use Without<IsResource> default query filters. ... oh, but you aren't actually checking that in UnsafeWorldCell::get_resource_with_ticks!

I think you need to check for the IsResource component consistently. Otherwise a malicious user could remove the component, and then get aliasing between queries with Without<IsResource> and resource access APIs.

It might help to have a method like UnsafeWorldCell::get_resource_entity(self, component_id: ComponentId) -> Option<UnsafeEntityCell> that will encapsulate doing the lookup on resource_entities and checking that the entity exists and has the component_id and IsResource components.

We may also want to cache the component_id of IsResource on the World somehow, or reserve a fixed value, in order to avoid the hash lookup from the TypeId.

Copy link
Contributor Author

@Trashtalk217 Trashtalk217 Oct 31, 2025

Choose a reason for hiding this comment

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

... a malicious user could ...

While all of the things you say are true: this is a game engine. Our users are game developers, not hackers. I'll look into this, but we can't guardrail every single corner of Bevy.

Copy link
Contributor

Choose a reason for hiding this comment

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

While all of the things you say are true: this is a game engine. Our users are game developers, not hackers. I'll look into this, but we can't guardrail every single corner of Bevy.

Hah! Fair :). Though maybe I should apply Hanlon's razor and say that a foolish user could do those things :).

If the consequences here were a panic or weird behavior then I wouldn't worry, but this can cause Undefined Behavior if we get it wrong, and I think Bevy is usually pretty careful not to allow UB in safe code.

I think it's really just the checks in Res and ResMut that are important for safety, though, since those need to not alias with our friend Query<EntityMut, Without<IsResource>>. But if we change those, then it would just be surprising if World::get_resource found a resource that Res did not.

A hypothetical future implementation of Res<R> in terms of Single<R, With<IsResource>> would also check for IsResource, of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After the changes to Res and ResMut I don't think that this should cause UB anymore. Btw: I'm not using Single<> there because cart felt that QueryState was unnecessarily large (it's two component ids now).

@Trashtalk217
Copy link
Contributor Author

Trashtalk217 commented Nov 2, 2025

Having IsResource as a default query filter

There has been much discussion with regards to IsResource being a default query filter (DFQ) on Discord. And after a request by cart, I decided to look at the impact of unmaking IsResource a DFQ. As a reminder: If B is a default query filter, then Without<B> is automatically added to a query. So Query<&A> quietly becomes Query<&A, Without<B>>.

IsResource is a DFQ

If IsResource is registered as a DFQ, virtually nothing changes compared to the status quo. Query<Entity> returns the same number of entities before and after transitioning to resource-as-components. The benefits of transparency are obvious, since we're not hiding anything. This is why we are mostly focused on the case where IsResource is not a DFQ.

IsResource is not a DFQ

This has the largest effect on broad queries. Broad queries access all entities, so if a system contains a broad query and also a resource, this creates a conflict that wasn't there before resources-as-components.

Examples of broad queries include:

  • Query<()>
  • Query<Entity>
  • Query<EntityMut>
  • Query<EntityRef>
  • Query<EntityMutExcept>
  • Query<EntityRefExcept>
  • Query<Option<&A>>

The first two are not really big deal. Since both access no components, they don't conflict with a Res<R> or a ResMut<R> in a system. The only thing that changes is the number of entities that they return, which - in tests in particular - can cause problems. See also #20207, #20248, #21685, and the last two commits for my work on this.

The last 5 queries can cause issues because they access all components on the queried entities, meaning that fn(q: Query<EntityMut>, r: Res<R> does conflict. When fixing this PR, the places where this showed up most prevalently were tests, again. After fixing those, there were three other places in Bevy, that demanded attention.

bevy_scene

bevy_scene, by extracting entities and resources separately, runs into problems with resources-as-components. Most of which are helped by manually filtering out resource entities where they may show up:

// dynamic_scene_builder.rs
pub fn extract_entities(mut self, entities: impl Iterator<Item = Entity>) -> Self {
    let resource_entities: Vec<Entity> = self.original_world.resource_entities().values().copied().collect();
    
    for entity in entities {
        // I know this is inefficient (not the point right now)
        if (resource_entities.contains(&entity) { continue; }
        // ..

bevy_animation

Since AnimationEntityMut shadows EntityMutExcept, it has the same problems. This is relevant for exactly one system: animate_targets:

// before
pub fn animate_targets(
    ...
    mut targets: Query<(Entity, &AnimationTargetId, &AnimatedBy, AnimationEntityMut)>,
    ...
) { ... }
// after
pub fn animate_targets(
    ...
    mut targets: Query<(Entity, &AnimationTargetId, &AnimatedBy, AnimationEntityMut), Without<IsResource>>,
    ...
) { ... }

Do note that AnimationEntityMut is pub so any user code that uses it, also has the same problem.

bevy_input_focus

A broad query is used in tab_navigation.rs:

pub struct TabNavigation<'w, 's> {
    // Query for tab groups.
    tabgroup_query: Query<'w, 's, (Entity, &'static TabGroup, &'static Children)>,
    // Query for tab indices.
    tabindex_query: Query<
        'w,
        's,
        (Entity, Option<&'static TabIndex>, Option<&'static Children>),
        Without<TabGroup>,
    >,
    // Query for parents.
    parent_query: Query<'w, 's, &'static ChildOf>,
}

Here, tabindex_query pulls in all entities, but there is no conflict and nothing needs to be changed except the test where tabindex_query.iter(&world).count() no longer returns the same number (this was already done in #21685).

Conclusion

Overall, there aren't many places where IsResource not being a DFQ causes problems. It's mostly a nuisance in test suites, and only sparsely happens in other Bevy code. This, of course, does not take into account how much users need to change in order to avoid conflicts, but this at least gives a sample.

@alice-i-cecile
Copy link
Member

Query<Option<&A>> is definitely the most concerning of these, but it's very rare for it to be used on its own without any other qualifying terms.

That said, (iteration over) broad queries should never occur in engine / library code outside of very niche applications like networking. These have a linear performance cost with the total number of entities, regardless of the type of entity, and should be refactored whenever possible.

Evaluation of the current cases:

  1. Scenes: justified use, scenes are inherently fine.
  2. Animation: we could trivially extend AnimationEntityMut to exclude resource entities and avoid breakage for end users as well.
  3. Input Focus: the query in question appears to be used for point queries only: won't have any linear performance consequences.

@andriyDev
Copy link
Contributor

@alice-i-cecile My initial reaction was also that Query<Option<&T>> was concerning, but then I realized, why wouldn't you just use Query<&T> and then filter out the error cases?

I also don't understand how that conflicts. We're not accessing broad data there, and if the component is missing we don't access anything on the resource entities. Sure we'll iterate through different entities but the shouldn't generate a conflict I think? @Trashtalk217 am I misunderstand here?

This was a very useful exploration though, thank you!

@Trashtalk217 Trashtalk217 requested a review from chescock November 3, 2025 02:34
@chescock
Copy link
Contributor

chescock commented Nov 3, 2025

For AnimationEntityMut: If we allow IsResource entities in that query, and then add the three resources used by the animate_targets system to the "except" list, is that enough to support animating resources? Is it useful to animate resources?

If generic code like that winds up being useful for resources without any other changes, that seems like an argument in favor of allowing IsResource entities by default!

}

#[test]
fn filtered_resource_reflect() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this test break? ... Oh, I see, ReflectResource no longer has a way to get a resource directly from a world!

ReflectComponent::get isn't an exact replacement for ReflectResource::get, since ReflectResource::get started from the world while ReflectComponent::get needed a specific entity.

I think we need to preserve that functionality somehow, so that reflection can look up resources through the resource_entities map. One option is to restore ReflectResource as its own thing, and another option is to add resource-like methods to ReflectComponent. If we do the latter, we should probably add #[deprecated] methods to ReflectResource to teach users about the name change.

Copy link
Contributor Author

@Trashtalk217 Trashtalk217 Nov 7, 2025

Choose a reason for hiding this comment

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

This is one of the few points where I decided to draw a line for the API. When implementing the v2, where resources were wrapped in ResourceComponent<...>, one major advantage was that any given struct can be annotated with both Resource and Component. But, as cart pointed out, this is confusing. If resources are 'just' components, how are they components in two ways?

A similar reasoning went into making ReflectResource shadow ReflectComponent exactly. Given that a Resource both derives the Resource and Component Trait, it's possible to both reflect(Component) as well as reflect(Resource).

This alone is kind of confusing, but in my opinion, it would be stranger if they both had a different interface.

Additionally, most of the time, the necessary entity data is nearby (see also the changes to bevy_remote), and reflection is deep enough into the internals that we can expect users to work with resources-as-components.

A better solution - in my opinion - is to have FilteredResources return FilteredEntityRefs instead of a FilteredResources which do work with reflect_resource.reflect(...), but that's not a change I'm making in this PR.

}

component_access_set.add_unfiltered_resource_read(component_id);
let mut filter = FilteredAccess::default();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to make a corresponding change to FilteredResources and FilteredResourcesMut so that they correctly conflict with queries.

The has_read/write_all_resources() branches would probably do filter.read_all_components(); filter.and_with(*is_resource_id); to declare access to all components on entities with IsResource, while the other branch would use the code you have here in a loop.

Alternately, it might make sense to modify add_unfiltered_resource_read/write() itself so that the code could be shared. I guess that would also affect NonSend, but requesting additional access like that should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I was able to change filtered_resource.rs and the builder structs to make this happen. Everything now doubly registers access, both as a component as well as a resource, but that can be streamlined later.

@Trashtalk217 Trashtalk217 requested a review from cart November 29, 2025 00:14
@Trashtalk217 Trashtalk217 added the S-Needs-SME Decision or review from an SME is required label Nov 29, 2025
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

I think this is on the right track!

ComponentDescriptor::new_resource::<T>()
})
}
self.register_component::<T>()
Copy link
Member

Choose a reason for hiding this comment

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

I think we should consider deprecating this in favor of register_component

/// * [`ComponentsRegistrator::register_non_send()`]
#[inline]
pub fn register_resource_with_descriptor(
pub fn register_non_send_with_descriptor(
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant with register_component_with_descriptor. We should consider removing it.

/// Backing storage for resources.
pub resources: Resources<true>,
/// Backing storage for `!Send` resources.
pub non_send_resources: Resources<false>,
Copy link
Member

Choose a reason for hiding this comment

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

In this new world, I think NonSend deserves a "reframe", as these are no longer "resources" in the same way. Ex: "NonSend data", or in this case Resources<false> -> NonSends

Copy link
Contributor

Choose a reason for hiding this comment

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

The current plan around NonSend was to just delete them and make users use thread locals. This is because we want to refactor the winit loop and accounting for moving the non send storage out of the world was making things too complicated. All the in engine uses have already been replaced with thread locals already. So we could just remove them if we want to merge this soon. But it might be better to deprecate them for 0.18 first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not familiar with the specifics of thread locals, but on the discord I have seen people vocal about keeping it. From what I've heard it's something few users need, but for specific usecases - like scripting - it's the indispensable. I am renaming doing the rename as suggested, but removing NonSendData is out of scope for this PR.

/// [`bevy_reflect::TypeRegistration::data`].
#[derive(Clone)]
pub struct ReflectResource(ReflectResourceFns);
pub struct ReflectResource(ReflectComponent);
Copy link
Member

Choose a reason for hiding this comment

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

People building reflection features should be able to assume that if a component implements Reflect and registers itself, that "component reflection" features will be present. Resources are components, so forcing reflection consumers to special case them when building features feels off.

I don't want to table this for later, because if we land this PR now, we would probably release 0.18 in a state where developers need to change their usage patterns, then change them again in 0.19.

However we might also want additional Resource reflection functionality / it does make sense to reflect(Resource). Additionally that ensures we don't break existing code. Buuut we also don't really want to require reflect(Resource, Component). Ideally reflect(Resource) could imply reflect(Component), but we don't currently have the functionality. I'll add that really quick, then you can take a dependency here.

Copy link
Member

@cart cart Dec 3, 2025

Choose a reason for hiding this comment

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

.is_some_and(ResourceData::is_present)
if let Some(entity) = self.resource_entities.get(component_id)
&& let Ok(entity_ref) = self.get_entity(*entity)
{
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking we can probably remove the IsResource check here. Functionally the "resource entities" cache is behaving as the source of truth here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking we can probably remove the IsResource check here. Functionally the "resource entities" cache is behaving as the source of truth here.

I think we need the check in Res and ResMut for soundness, and then we'll want it here for consistency. My thought process is:

We want to be able to use ResMut<R> and EntityMut in the same system, so we want to ensure that Query<EntityMut, Without<IsResource>> does not conflict with ResMut<R>.

The IsResource component can be removed in safe code. Doing that is weird, so we can panic or do something weird, but we can't cause UB.

Therefore, Res<R> and ResMut<R> must ignore resource entities that don't have the IsResource component. Otherwise they could cause aliasing with a Query<EntityMut, Without<IsResource>>.

And if Res<R> fails to find a resource, then contains_resource::<R> should return false for consistency.


A slightly different approach would be to remove an entity from the "resource entities" cache if its IsResource component is removed, probably by using an on_replace hook on IsResource. That would ensure that the cache was the source of truth. You'd need to get the ComponentId somehow, so you'd probably want to store it in the IsResource component. But storing it there might be useful anyway for dynamic resource access!


/// Create a new dynamic scene from a given world.
pub fn from_world(world: &World) -> Self {
let resource_entities: Vec<Entity> = world.resource_entities().values().copied().collect();
Copy link
Member

Choose a reason for hiding this comment

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

I'm not in love with this whole "filter out resource entities only to insert the resource entities" approach. But it will require rethinking how "resource filters" work so I'm cool with tabling it.

continue;
}

if resource_entities.contains(&entity) {
Copy link
Member

Choose a reason for hiding this comment

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

This isn't going to cut it preformance-wise. We might have hundreds of thousands of entities and hundreds of resources.

Resources are entities and extract_entities as an operation shouldn't care about which entities are passed in. It should be the responsibility of the caller to determine what entities should be extracted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would lead to a possible conflict between extract_resources and extract_entities. What happens if you both extract a resource as well as its entity? Both would be in the scene file, so when reading that back in, which should get priority?

I really wanted to tackle the bevy_scene seperately from this PR, maybe we can put up a warning in the method docs for now?

world.spawn_empty().id()
};

reflect_resource.copy(
Copy link
Member

Choose a reason for hiding this comment

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

This notably won't carry over any other components attached to the resource. We're losing one of the primary benefits of the "resources as entities" thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really wanted to redo bevy_scene completely. However, after your review of #21346, where you didn't want to change the scene format in a major way, I mostly reverted all of the changes. I've come to the conclusion that I really want a scene file to 'just be entities' and maybe some metadata.

// everything is an entity and just goes here, including resources
4294967297: (
  components: {
    "bevy_ecs::name::Name": "joe",
    "bevy_transform::components::global_transform::GlobalTransform": ((1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0)),
    // ...
    "scene::ComponentB": (
      value: "hello",
    ),
  },
),
4294967298: (
  components: {
    "scene::ResourceA": (
      x: 3.0,
      y: 4.0,
    ),
   "bevy_ecs::resource::Is_Resource": (),
  },
),

We would have to solve the issue of two resources with the same ComponentId being loaded from two different scene files, but I think that's managable. With this, carrying over other components is easy.

In any case, I expected the design work to be in some other PR in the same cycle, and instead opted for keeping it as-is.

component_access_set.add_unfiltered_resource_read(component_id);
let mut filter = FilteredAccess::default();
filter.add_component_read(component_id);
filter.add_resource_read(component_id);
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer it if we took the time to simplify Access in this PR. It is a key piece of this port, and I'm personally more interested in landing this "right" than landing it quick.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer it if we took the time to simplify Access in this PR. It is a key piece of this port, and I'm personally more interested in landing this "right" than landing it quick.

The tricky part here is that WorldQuery types like AccessChanged<A> can declare resource access (#16843). That resource access is outside of the entities matched by the query, so if we remove the separation between component and resource access, then we need a different way to express that. One option is to support queries having multiple FilteredAccesses, such as by doing #21557.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can take a look at this again, but most of the 'why didn't you also do this in this PR' comes down to those things being non-trivial and me not wanting to balloon this PR more than necessary.

I will look at this again tomorrow though.

@cart cart mentioned this pull request Dec 3, 2025
github-merge-queue bot pushed a commit that referenced this pull request Dec 3, 2025
# Objective

The proposed ["Resources as
entities"](#20934 ) PR makes
Resource implement Component. This creates a situation where we need to
do:

```rust
#[derive(Resource, Reflect)]
#[reflect(Resource, Component)]
struct Thing;
```

I'm of the mind that we should add the ability for TypeData to have
"dependencies". Ex `reflect(Resource)` implies `reflect(Component)`. For
"subtrait" types, I think supporting this is logical / users would
appreciate it.

## Solution

1. Add a new `FromType<T>::insert_dependencies` function with a default
impl (which makes this a non-breaking change). This does kind of
overload the `FromType` trait (ex: a name like `GetTypeData` might be
better with this new context), but this is a pretty niche trait / piece
of functionality, and I like the idea of not breaking people.
2. Add a new `TypeRegistration::register_type_data<T, V>` function,
which initializes the TypeData `T` for a given type `V` , inserts that
type data, and also inserts any dependent type data using
`insert_dependencies`.
3. Adjust the `Reflect` macro to use `register_type_data` instead of
`insert(FromType::<Self>::from_type())`

This makes it possible to do the following:

```rust
impl<R: Resource + FromReflect + TypePath> FromType<R> for ReflectResource {
    fn from_type() -> Self {
        ReflectResource
    }

    fn insert_dependencies(type_registration: &mut TypeRegistration) {
        type_registration.register_type_data::<ReflectComponent, R>();
    }
}
```

Which then allows dropping `reflect(Component)`:

```rust
#[derive(Resource, Reflect)]
#[reflect(Resource)]
struct Thing;
```

## Testing

I added a unit test 😜
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Needs-SME Decision or review from an SME is required

Projects

None yet

Development

Successfully merging this pull request may close these issues.