Commit graph

11 commits

Author SHA1 Message Date
Kanabenki
569e2ac80f
Make builder types take and return Self (#10001)
# Objective

Closes #9955.

Use the same interface for all "pure" builder types: taking and
returning `Self` (and not `&mut Self`).

## Solution

Changed `DynamicSceneBuilder`, `SceneFilter` and `TableBuilder` to take
and return `Self`.

## Changelog

### Changed

- `DynamicSceneBuilder` and `SceneBuilder` methods in `bevy_ecs` now
take and return `Self`.

## Migration guide

When using `bevy_ecs::DynamicSceneBuilder` and `bevy_ecs::SceneBuilder`,
instead of binding the builder to a variable, directly use it. Methods
on those types now consume `Self`, so you will need to re-bind the
builder if you don't `build` it immediately.

Before:
```rust
let mut scene_builder = DynamicSceneBuilder::from_world(&world);
let scene = scene_builder.extract_entity(a).extract_entity(b).build();
```

After:
 ```rust
let scene = DynamicSceneBuilder::from_world(&world)
    .extract_entity(a)
    .extract_entity(b)
    .build();
```
2023-10-09 19:46:17 +00:00
Gino Valente
d96933ad9c
bevy_scene: Add SceneFilter (#6793)
# Objective

Currently, `DynamicScene`s extract all components listed in the given
(or the world's) type registry. This acts as a quasi-filter of sorts.
However, it can be troublesome to use effectively and lacks decent
control.

For example, say you need to serialize only the following component over
the network:

```rust
#[derive(Reflect, Component, Default)]
#[reflect(Component)]
struct NPC {
  name: Option<String>
}
```

To do this, you'd need to:
1. Create a new `AppTypeRegistry`
2. Register `NPC`
3. Register `Option<String>`

If we skip Step 3, then the entire scene might fail to serialize as
`Option<String>` requires registration.

Not only is this annoying and easy to forget, but it can leave users
with an impossible task: serializing a third-party type that contains
private types.

Generally, the third-party crate will register their private types
within a plugin so the user doesn't need to do it themselves. However,
this means we are now unable to serialize _just_ that type— we're forced
to allow everything!

## Solution

Add the `SceneFilter` enum for filtering components to extract.

This filter can be used to optionally allow or deny entire sets of
components/resources. With the `DynamicSceneBuilder`, users have more
control over how their `DynamicScene`s are built.

To only serialize a subset of components, use the `allow` method:
```rust
let scene = builder
  .allow::<ComponentA>()
  .allow::<ComponentB>()
  .extract_entity(entity)
  .build();
```

To serialize everything _but_ a subset of components, use the `deny`
method:
```rust
let scene = builder
  .deny::<ComponentA>()
  .deny::<ComponentB>()
  .extract_entity(entity)
  .build();
```

Or create a custom filter:
```rust
let components = HashSet::from([type_id]);

let filter = SceneFilter::Allowlist(components);
// let filter = SceneFilter::Denylist(components);

let scene = builder
  .with_filter(Some(filter))
  .extract_entity(entity)
  .build();
```

Similar operations exist for resources:

<details>
<summary>View Resource Methods</summary>

To only serialize a subset of resources, use the `allow_resource`
method:
```rust
let scene = builder
  .allow_resource::<ResourceA>()
  .extract_resources()
  .build();
```

To serialize everything _but_ a subset of resources, use the
`deny_resource` method:
```rust
let scene = builder
  .deny_resource::<ResourceA>()
  .extract_resources()
  .build();
```

Or create a custom filter:
```rust
let resources = HashSet::from([type_id]);

let filter = SceneFilter::Allowlist(resources);
// let filter = SceneFilter::Denylist(resources);

let scene = builder
  .with_resource_filter(Some(filter))
  .extract_resources()
  .build();
```

</details>

### Open Questions

- [x] ~~`allow` and `deny` are mutually exclusive. Currently, they
overwrite each other. Should this instead be a panic?~~ Took @soqb's
suggestion and made it so that the opposing method simply removes that
type from the list.
- [x] ~~`DynamicSceneBuilder` extracts entity data as soon as
`extract_entity`/`extract_entities` is called. Should this behavior
instead be moved to the `build` method to prevent ordering mixups (e.g.
`.allow::<Foo>().extract_entity(entity)` vs
`.extract_entity(entity).allow::<Foo>()`)? The tradeoff would be
iterating over the given entities twice: once at extraction and again at
build.~~ Based on the feedback from @Testare it sounds like it might be
better to just keep the current functionality (if anything we can open a
separate PR that adds deferred methods for extraction, so the
choice/performance hit is up to the user).
- [ ] An alternative might be to remove the filter from
`DynamicSceneBuilder` and have it as a separate parameter to the
extraction methods (either in the existing ones or as added
`extract_entity_with_filter`-type methods). Is this preferable?
- [x] ~~Should we include constructors that include common types to
allow/deny? For example, a `SceneFilter::standard_allowlist` that
includes things like `Parent` and `Children`?~~ Consensus suggests we
should. I may split this out into a followup PR, though.
- [x] ~~Should we add the ability to remove types from the filter
regardless of whether an allowlist or denylist (e.g.
`filter.remove::<Foo>()`)?~~ See the first list item
- [x] ~~Should `SceneFilter` be an enum? Would it make more sense as a
struct that contains an `is_denylist` boolean?~~ With the added
`SceneFilter::None` state (replacing the need to wrap in an `Option` or
rely on an empty `Denylist`), it seems an enum is better suited now
- [x] ~~Bikeshed: Do we like the naming convention? Should we instead
use `include`/`exclude` terminology?~~ Sounds like we're sticking with
`allow`/`deny`!
- [x] ~~Does this feature need a new example? Do we simply include it in
the existing one (maybe even as a comment?)? Should this be done in a
followup PR instead?~~ Example will be added in a followup PR

### Followup Tasks

- [ ] Add a dedicated `SceneFilter` example
- [ ] Possibly add default types to the filter (e.g. deny things like
`ComputedVisibility`, allow `Parent`, etc)

---

## Changelog

- Added the `SceneFilter` enum for filtering components and resources
when building a `DynamicScene`
- Added methods:
  - `DynamicSceneBuilder::with_filter`
  - `DynamicSceneBuilder::allow`
  - `DynamicSceneBuilder::deny`
  - `DynamicSceneBuilder::allow_all`
  - `DynamicSceneBuilder::deny_all`
  - `DynamicSceneBuilder::with_resource_filter`
  - `DynamicSceneBuilder::allow_resource`
  - `DynamicSceneBuilder::deny_resource`
  - `DynamicSceneBuilder::allow_all_resources`
  - `DynamicSceneBuilder::deny_all_resources`
- Removed methods:
  - `DynamicSceneBuilder::from_world_with_type_registry`
- `DynamicScene::from_scene` and `DynamicScene::from_world` no longer
require an `AppTypeRegistry` reference

## Migration Guide

- `DynamicScene::from_scene` and `DynamicScene::from_world` no longer
require an `AppTypeRegistry` reference:
  ```rust
  // OLD
  let registry = world.resource::<AppTypeRegistry>();
  let dynamic_scene = DynamicScene::from_world(&world, registry);
  // let dynamic_scene = DynamicScene::from_scene(&scene, registry);
  
  // NEW
  let dynamic_scene = DynamicScene::from_world(&world);
  // let dynamic_scene = DynamicScene::from_scene(&scene);
  ```

- Removed `DynamicSceneBuilder::from_world_with_type_registry`. Now the
registry is automatically taken from the given world:
  ```rust
  // OLD
  let registry = world.resource::<AppTypeRegistry>();
let builder = DynamicSceneBuilder::from_world_with_type_registry(&world,
registry);
  
  // NEW
  let builder = DynamicSceneBuilder::from_world(&world);
  ```
2023-07-06 21:04:26 +00:00
Nicola Papale
0294bb191d
Move AppTypeRegistry to bevy_ecs (#8901)
# Objective

- Use `AppTypeRegistry` on API defined in `bevy_ecs`
(https://github.com/bevyengine/bevy/pull/8895#discussion_r1234748418)

A lot of the API on `Reflect` depends on a registry. When it comes to
the ECS. We should use `AppTypeRegistry` in the general case.

This is however impossible in `bevy_ecs`, since `AppTypeRegistry` is
defined in `bevy_app`.

## Solution

- Move `AppTypeRegistry` resource definition from `bevy_app` to
`bevy_ecs`
- Still add the resource in the `App` plugin, since bevy_ecs itself
doesn't know of plugins

Note that `bevy_ecs` is a dependency of `bevy_app`, so nothing
revolutionary happens.

## Alternative

- Define the API as a trait in `bevy_app` over `bevy_ecs`. (though this
prevents us from using bevy_ecs internals)
- Do not rely on `AppTypeRegistry` for the API in question, requring
users to extract themselves the resource and pass it to the API methods.

---

## Changelog

- Moved `AppTypeRegistry` resource definition from `bevy_app` to
`bevy_ecs`

## Migration Guide

- If you were **not** using a `prelude::*` to import `AppTypeRegistry`,
you should update your imports:

```diff
- use bevy::app::AppTypeRegistry;
+ use bevy::ecs::reflect::AppTypeRegistry
```
2023-06-21 17:25:01 +00:00
Illiux
eebc92a7d4
Make scene handling of entity references robust (#7335)
# Objective

- Handle dangling entity references inside scenes
- Handle references to entities with generation > 0 inside scenes
- Fix a latent bug in `Parent`'s `MapEntities` implementation, which
would, if the parent was outside the scene, cause the scene to be loaded
into the new world with a parent reference potentially pointing to some
random entity in that new world.
- Fixes #4793 and addresses #7235 

## Solution

- DynamicScenes now identify entities with a `Entity` instead of a u32,
therefore including generation
- `World` exposes a new `reserve_generations` function that despawns an
entity and advances its generation by some extra amount.
- `MapEntities` implementations have a new `get_or_reserve` function
available that will always return an `Entity`, establishing a new
mapping to a dead entity when the entity they are called with is not in
the `EntityMap`. Subsequent calls with that same `Entity` will return
the same newly created dead entity reference, preserving equality
semantics.
- As a result, after loading a scene containing references to dead
entities (or entities otherwise outside the scene), those references
will all point to different generations on a single entity id in the new
world.

---

## Changelog

### Changed
- In serialized scenes, entities are now identified by a u64 instead of
a u32.
- In serialized scenes, components with entity references now have those
references serialize as u64s instead of structs.
### Fixed
- Scenes containing components with entity references will now
deserialize and add to a world reliably.

## Migration Guide

- `MapEntities` implementations must change from a `&EntityMap`
parameter to a `&mut EntityMapper` parameter and can no longer return a
`Result`. Finally, they should switch from calling `EntityMap::get` to
calling `EntityMapper::get_or_reserve`.

---------

Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
2023-05-01 15:49:27 +00:00
Francesco
7b38de0a64
(De) serialize resources in scenes (#6846)
# Objective

Co-Authored-By: davier
[bricedavier@gmail.com](mailto:bricedavier@gmail.com)
Fixes #3576.
Adds a `resources` field in scene serialization data to allow
de/serializing resources that have reflection enabled.

## Solution

Most of this code is taken from a previous closed PR:
https://github.com/bevyengine/bevy/pull/3580. Most of the credit goes to
@Davier , what I did was mostly getting it to work on the latest main
branch of Bevy, along with adding a few asserts in the currently
existing tests to be sure everything is working properly.

This PR changes the scene format to include resources in this way:
```
(
  resources: {
    // List of resources here, keyed by resource type name.
  },
  entities: [
    // Previous scene format here
  ],
)
```

An example taken from the tests:
```
(
  resources: {
    "bevy_scene::serde::tests::MyResource": (
      foo: 123,
    ),
  },
  entities: {
    // Previous scene format here
  },
)
```
For this, a `resources` fields has been added on the `DynamicScene` and
the `DynamicSceneBuilder` structs. The latter now also has a method
named `extract_resources` to properly extract the existing resources
registered in the local type registry, in a similar way to
`extract_entities`.


---

## Changelog


Added: Reflect resources registered in the type registry used by dynamic
scenes will now be properly de/serialized in scene data.

## Migration Guide

Since the scene format has been changed, the user may not be able to use
scenes saved prior to this PR due to the `resources` scene field being
missing. ~~To preserve backwards compatibility, I will try to make the
`resources` fully optional so that old scenes can be loaded without
issue.~~

## TODOs

- [x] I may have to update a few doc blocks still referring to dynamic
scenes as mere container of entities, since they now include resources
as well.
- [x] ~~I want to make the `resources` key optional, as specified in the
Migration Guide, so that old scenes will be compatible with this
change.~~ Since this would only be trivial for ron format, I think it
might be better to consider it in a separate PR/discussion to figure out
if it could be done for binary serialization too.
- [x] I suppose it might be a good idea to add a resources in the scene
example so that users will quickly notice they can serialize resources
just like entities.

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-03-20 21:17:02 +00:00
Jakub Łabor
e1d741aa19 bevy_ecs: ReflectComponentFns without World (#7206)
# Objective

Ability to use `ReflectComponent` methods in dynamic type contexts with no access to `&World`.

This problem occurred to me when wanting to apply reflected types to an entity where the `&World` reference was already consumed by query iterator leaving only `EntityMut`.

## Solution

- Remove redundant `EntityMut` or `EntityRef` lookup from `World` and `Entity` in favor of taking `EntityMut` directly in `ReflectComponentFns`.
- Added `RefectComponent::contains` to determine without panic whether `apply` can be used.

## Changelog

- Changed function signatures of `ReflectComponent` methods, `apply`, `remove`, `contains`, and `reflect`.

## Migration Guide

- Call `World::entity` before calling into the changed `ReflectComponent` methods, most likely user already has a `EntityRef` or `EntityMut` which was being queried redundantly.
2023-02-03 05:53:58 +00:00
Rob Parrett
3dd8b42f72 Fix various typos (#7096)
I stumbled across a typo in some docs. Fixed some more while I was in there.
2023-01-06 00:43:30 +00:00
François
e8b28547bf Cleanup dynamic scene before building (#6254)
# Objective

- Dynamic scene builder can build scenes without components, if they didn't have any matching the type registry
- Those entities are not really useful in the final `DynamicScene`

## Solution

- Add a method `remove_empty_entities` that will remove empty entities. It's not called by default when calling `build`, I'm not sure if that's a good idea or not.
2022-12-20 16:16:58 +00:00
Edvin Kjell
a8a62fcf3d [Fixes #6059] `Entity`'s “ID” should be named “index” instead (#6107)
# Objective

Fixes #6059, changing all incorrect occurrences of ``id`` in the ``entity`` module to ``index``:

* struct level documentation,
* ``id`` struct field,
* ``id`` method and its documentation.

## Solution

Renaming and verifying using CI. 


Co-authored-by: Edvin Kjell <43633999+Edwox@users.noreply.github.com>
2022-11-02 15:19:50 +00:00
Gino Valente
284b1f1302 bevy_scene: Stabilize entity order in DynamicSceneBuilder (#6382)
# Objective

Currently, `DynamicSceneBuilder` keeps track of entities via a `HashMap`. This has an unintended side-effect in that, when building the full `DynamicScene`, we aren't guaranteed any particular order. 

In other words, inserting Entity A then Entity B can result in either `[A, B]` or `[B, A]`. This can be rather annoying when running tests on scenes generated via the builder as it will work sometimes but not other times. There's also the potential that this might unnecessarily clutter up VCS diffs for scene files (assuming they had an intentional order).

## Solution

Store `DynamicSceneBuilder`'s entities in a `Vec` rather than a `HashMap`.

---

## Changelog

* Stablized entity order in `DynamicSceneBuilder` (0.9.0-dev)
2022-10-27 23:38:03 +00:00
François
ccf7c65a78 dynamic scene builder (#6227)
# Objective

- make it easier to build dynamic scenes

## Solution

- add a builder to create a dynamic scene from a world. it can extract an entity or an iterator of entities
- alternative to #6013, leaving the "hierarchy iteration" part to #6185 which does it better
- alternative to #6004 
- using a builder makes it easier to chain several extractions
2022-10-12 01:48:16 +00:00