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

Make builder types take and return Self #10001

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Make builder types take and return Self #10001

merged 1 commit into from
Oct 9, 2023

Conversation

Kanabenki
Copy link
Contributor

@Kanabenki Kanabenki commented Oct 2, 2023 •
edited
Loading

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:

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

After:

let scene = DynamicSceneBuilder::from_world(&world)
   .extract_entity(a)
   .extract_entity(b)
   .build();

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Scenes Serialized ECS data stored on the disk M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Oct 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

james7132
james7132 approved these changes Oct 3, 2023
View reviewed changes
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

The changes to TableBuilder are really only relevant when new archetypes are introduced, so it's probably not too big of a perf concern.

@james7132 james7132 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 Oct 5, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 9, 2023
Merged via the queue into bevyengine:main with commit 569e2ac Oct 9, 2023
22 checks passed
regnarock pushed a commit to regnarock/bevy that referenced this pull request Oct 13, 2023
@Kanabenki
Make builder types take and return Self (bevyengine#10001)
11e99f2 
# Objective

Closes bevyengine#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();
```
ameknite pushed a commit to ameknite/bevy that referenced this pull request Nov 6, 2023
@Kanabenki @ameknite
Make builder types take and return Self (bevyengine#10001)
80fd94e 
# Objective

Closes bevyengine#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();
```
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
@Kanabenki
Make builder types take and return Self (bevyengine#10001)
3b31ce1 
# Objective

Closes bevyengine#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();
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@james7132 james7132 james7132 approved these changes

@killercup killercup killercup approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Standardize self usage for builder-like types
4 participants
@Kanabenki @killercup @james7132 @alice-i-cecile