explicitly mention component in methods on DynamicSceneBuilder

[atornity]

Objective

The method names on DynamicSceneBuilder are misleading. Specifically, deny_all and allow_all implies everything will be denied/allowed, including all components and resources. In reality, these methods only apply to components (which is mentioned in the docs).

Solution

• change deny_all and allow_all to deny_all_components and allow_all_components
• also, change the remaining methods to mention component where it makes sense

We could also add the deny_all and allow_all methods back later, only this time, they would deny/allow both resources and components.

Showcase

Before

let builder = DynamicSceneBuilder::from_world(world)
    .deny_all()
    .deny_all_resources()
    .allow::<MyComponent>();

After

let builder = DynamicSceneBuilder::from_world(world)
    .deny_all_components()
    .deny_all_resources()
    .allow_component::<MyComponent>();

Migration Guide

DynamicSceneBuilder::allow_all and deny_all now set resource accesses, not just components. To return to the previous behavior, use the new allow_all_components or deny_all_components methods.

The following methods for DynamicSceneBuilder have been renamed:

• with_filter -> with_component_filter
• allow -> allow_component
• deny -> deny_component

[alice-i-cecile]

We could also add the deny_all and allow_all methods back later, only this time, they would deny/allow both resources and components.

Agreed, I'd like this functionality (and allow_all_resources) later, but this rename is a much clearer first step.

[atornity]

Agreed, I'd like this functionality (and allow_all_resources) later, but this rename is a much clearer first step.

we actually already have deny/allow_all_resources

[janhohenheim]

Changing deny_all and allow_all makes sense to me, but why rename deny and allow? Due to static typing, there is no potential confusion there. And in the future, I'd prefer the deny and allow methods to accept both components and resources over duplicating the API for resources. Thoughts, @alice-i-cecile?

The filter changes seem good to me, as there is no static typing keeping you from adding resources to it, but I haven't used that part of the API, so take my comment with a grain of salt.

[atornity]

why rename deny and allow? [..] And in the future, I'd prefer the deny and allow methods to accept both components and resources over duplicating the API for resources.

I only renamed deny and allow for consistency with the other changes + the resource counterpart, I agree that there's not real problem with keeping deny and allow as it was.

I would definitely prefer deny and allow to accept both components and resources as well.

I could change that part of the pr back if you'd like, although I still personally prefer it this way for consistency.

[atornity]

does anyone know how I can run the doc test locally?

[alice-i-cecile]

I would definitely prefer deny and allow to accept both components and resources as well.

I don't think this is possible in a type-safe way. You could use ComponentId, or use the Any trait to generate one, but then you can deny things that can't possibly be components or resources. We would need a trait upstream of both Component and Resource.

It would also block both components and resources of that type when both exist, which is confusing. LWIM's InputMap implements both for example.

As a result, I think it's clearer / more future-proof to be explicit about components vs resources.

[janhohenheim]

Thanks for the explanation! I'm convinced then :)

[alice-i-cecile]

Note: I've combined the migration guide from #15222 into this PR to make for easier preparation of the final notes.