Skip to content

Commit

Permalink
Merge branch 'main' into bevy_ptr_align_const
Browse files Browse the repository at this point in the history
  • Loading branch information
mysteriouslyseeing authored Feb 28, 2025
2 parents 08fc7c2 + e43b28c commit b688398
Show file tree
Hide file tree
Showing 46 changed files with 1,000 additions and 355 deletions.
41 changes: 36 additions & 5 deletions .github/workflows/example-run-report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jobs:
timeout-minutes: 30
outputs:
branch-name: ${{ steps.branch-name.outputs.result }}
pr-number: ${{ steps.pr-number.outputs.result }}
steps:
- name: "Download artifact"
id: find-artifact
Expand Down Expand Up @@ -59,11 +60,11 @@ jobs:
- name: branch name
id: branch-name
run: |
if [ -f PR ]; then
echo "result=PR-$(cat PR)-${{ github.event.workflow_run.head_branch }}" >> $GITHUB_OUTPUT
else
echo "result=${{ github.event.workflow_run.head_branch }}" >> $GITHUB_OUTPUT
fi
echo "result=PR-$(cat PR)-${{ github.event.workflow_run.head_branch }}" >> $GITHUB_OUTPUT
- name: PR number
id: pr-number
run: |
echo "result=$(cat PR)" >> $GITHUB_OUTPUT
compare-macos-screenshots:
name: Compare macOS screenshots
Expand All @@ -75,3 +76,33 @@ jobs:
artifact: screenshots-macos
os: macos
secrets: inherit

comment-on-pr:
name: Comment on PR
runs-on: ubuntu-latest
needs: [make-macos-screenshots-available, compare-macos-screenshots]
if: ${{ always() && needs.compare-macos-screenshots.result == 'failure' }}
steps:
- uses: actions/checkout@v4
- name: "Check if PR already has label"
id: check-label
env:
PR: ${{ needs.make-macos-screenshots-available.outputs.pr-number }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
if [[ `gh api --jq '.labels.[].name' /repos/bevyengine/bevy/pulls/$PR` =~ "S-Rendering-Change" ]]
then
echo "result=true" >> $GITHUB_OUTPUT
else
echo "result=false" >> $GITHUB_OUTPUT
fi
- name: "Comment on PR"
if: ${{ steps.check-label.outputs.result == 'false' }}
env:
PROJECT: B04F67C0-C054-4A6F-92EC-F599FEC2FD1D
PR: ${{ needs.make-macos-screenshots-available.outputs.pr-number }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
LF=$'\n'
COMMENT_BODY="Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! ${LF}You can review it at https://pixel-eagle.com/project/$PROJECT?filter=PR-$PR ${LF} ${LF}If it's expected, please add the S-Deliberate-Rendering-Change label."
gh issue comment $PR --body "$COMMENT_BODY"
4 changes: 2 additions & 2 deletions benches/benches/bevy_ecs/entity_cloning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ fn bench_clone_hierarchy<B: Bundle + Default + GetTypeRegistration>(

hierarchy_level.clear();

for parent_id in current_hierarchy_level {
for parent in current_hierarchy_level {
for _ in 0..children {
let child_id = world.spawn((B::default(), ChildOf(parent_id))).id();
let child_id = world.spawn((B::default(), ChildOf { parent })).id();
hierarchy_level.push(child_id);
}
}
Expand Down
5 changes: 4 additions & 1 deletion benches/benches/bevy_math/bezier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ use criterion::{
criterion_group!(benches, segment_ease, curve_position, curve_iter_positions);

fn segment_ease(c: &mut Criterion) {
let segment = black_box(CubicSegment::new_bezier(vec2(0.25, 0.1), vec2(0.25, 1.0)));
let segment = black_box(CubicSegment::new_bezier_easing(
vec2(0.25, 0.1),
vec2(0.25, 1.0),
));

c.bench_function(bench!("segment_ease"), |b| {
let mut t = 0;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use bevy_ecs::prelude::*;
use std::cmp::Ordering;

#[derive(Component)]
struct A(usize);

fn system(mut query: Query<&mut A>) {
let iter = query.iter_mut();
let mut stored: Option<&A> = None;
let mut sorted = iter.sort_by::<&A>(|left, _right| {
// Try to smuggle the lens item out of the closure.
stored = Some(left);
//~^ E0521
Ordering::Equal
});
let r: &A = stored.unwrap();
let m: &mut A = &mut sorted.next().unwrap();
assert!(std::ptr::eq(r, m));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error[E0521]: borrowed data escapes outside of closure
--> tests/ui\system_query_iter_sort_lifetime_safety.rs:12:9
|
9 | let mut stored: Option<&A> = None;
| ---------- `stored` declared here, outside of the closure body
10 | let mut sorted = iter.sort_by::<&A>(|left, _right| {
| ---- `left` is a reference that is only valid in the closure body
11 | // Try to smuggle the lens item out of the closure.
12 | stored = Some(left);
| ^^^^^^^^^^^^^^^^^^^ `left` escapes the closure body here
134 changes: 91 additions & 43 deletions crates/bevy_ecs/macros/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use syn::{
punctuated::Punctuated,
spanned::Spanned,
token::{Comma, Paren},
Data, DataStruct, DeriveInput, ExprClosure, ExprPath, Fields, Ident, Index, LitStr, Member,
Path, Result, Token, Visibility,
Data, DataStruct, DeriveInput, ExprClosure, ExprPath, Field, Fields, Ident, Index, LitStr,
Member, Path, Result, Token, Visibility,
};

pub fn derive_event(input: TokenStream) -> TokenStream {
Expand Down Expand Up @@ -260,6 +260,18 @@ fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> T
Data::Struct(DataStruct { fields, .. }) => {
let mut visited_fields = Vec::new();
let mut visited_indices = Vec::new();

if is_relationship {
let field = match relationship_field(fields, "VisitEntities", fields.span()) {
Ok(f) => f,
Err(e) => return e.to_compile_error(),
};

match field.ident {
Some(ref ident) => visited_fields.push(ident.clone()),
None => visited_indices.push(Index::from(0)),
}
}
match fields {
Fields::Named(fields) => {
for field in &fields.named {
Expand All @@ -276,9 +288,7 @@ fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> T
}
Fields::Unnamed(fields) => {
for (index, field) in fields.unnamed.iter().enumerate() {
if index == 0 && is_relationship {
visited_indices.push(Index::from(0));
} else if field
if field
.attrs
.iter()
.any(|a| a.meta.path().is_ident(ENTITIES_ATTR))
Expand All @@ -289,7 +299,6 @@ fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> T
}
Fields::Unit => {}
}

if visited_fields.is_empty() && visited_indices.is_empty() {
TokenStream2::new()
} else {
Expand Down Expand Up @@ -651,25 +660,24 @@ fn derive_relationship(
let Some(relationship) = &attrs.relationship else {
return Ok(None);
};
const RELATIONSHIP_FORMAT_MESSAGE: &str = "Relationship derives must be a tuple struct with the only element being an EntityTargets type (ex: ChildOf(Entity))";
if let Data::Struct(DataStruct {
fields: Fields::Unnamed(unnamed_fields),
let Data::Struct(DataStruct {
fields,
struct_token,
..
}) = &ast.data
{
if unnamed_fields.unnamed.len() != 1 {
return Err(syn::Error::new(ast.span(), RELATIONSHIP_FORMAT_MESSAGE));
}
if unnamed_fields.unnamed.first().is_none() {
return Err(syn::Error::new(
struct_token.span(),
RELATIONSHIP_FORMAT_MESSAGE,
));
}
} else {
return Err(syn::Error::new(ast.span(), RELATIONSHIP_FORMAT_MESSAGE));
else {
return Err(syn::Error::new(
ast.span(),
"Relationship can only be derived for structs.",
));
};
let field = relationship_field(fields, "Relationship", struct_token.span())?;

let relationship_member: Member = field.ident.clone().map_or(Member::from(0), Member::Named);

let members = fields
.members()
.filter(|member| member != &relationship_member);

let struct_name = &ast.ident;
let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();
Expand All @@ -682,12 +690,15 @@ fn derive_relationship(

#[inline(always)]
fn get(&self) -> #bevy_ecs_path::entity::Entity {
self.0
self.#relationship_member
}

#[inline]
fn from(entity: #bevy_ecs_path::entity::Entity) -> Self {
Self(entity)
Self {
#(#members: core::default::Default::default(),),*
#relationship_member: entity
}
}
}
}))
Expand All @@ -702,30 +713,29 @@ fn derive_relationship_target(
return Ok(None);
};

const RELATIONSHIP_TARGET_FORMAT_MESSAGE: &str = "RelationshipTarget derives must be a tuple struct with the first element being a private RelationshipSourceCollection (ex: Children(Vec<Entity>))";
let collection = if let Data::Struct(DataStruct {
fields: Fields::Unnamed(unnamed_fields),
let Data::Struct(DataStruct {
fields,
struct_token,
..
}) = &ast.data
{
if let Some(first) = unnamed_fields.unnamed.first() {
if first.vis != Visibility::Inherited {
return Err(syn::Error::new(first.span(), "The collection in RelationshipTarget must be private to prevent users from directly mutating it, which could invalidate the correctness of relationships."));
}
first.ty.clone()
} else {
return Err(syn::Error::new(
struct_token.span(),
RELATIONSHIP_TARGET_FORMAT_MESSAGE,
));
}
} else {
else {
return Err(syn::Error::new(
ast.span(),
RELATIONSHIP_TARGET_FORMAT_MESSAGE,
"RelationshipTarget can only be derived for structs.",
));
};
let field = relationship_field(fields, "RelationshipTarget", struct_token.span())?;

if field.vis != Visibility::Inherited {
return Err(syn::Error::new(field.span(), "The collection in RelationshipTarget must be private to prevent users from directly mutating it, which could invalidate the correctness of relationships."));
}
let collection = &field.ty;

let relationship_member = field.ident.clone().map_or(Member::from(0), Member::Named);

let members = fields
.members()
.filter(|member| member != &relationship_member);

let relationship = &relationship_target.relationship;
let struct_name = &ast.ident;
Expand All @@ -739,18 +749,56 @@ fn derive_relationship_target(

#[inline]
fn collection(&self) -> &Self::Collection {
&self.0
&self.#relationship_member
}

#[inline]
fn collection_mut_risky(&mut self) -> &mut Self::Collection {
&mut self.0
&mut self.#relationship_member
}

#[inline]
fn from_collection_risky(collection: Self::Collection) -> Self {
Self(collection)
Self {
#(#members: core::default::Default::default(),),*
#relationship_member: collection
}
}
}
}))
}

/// Returns the field with the `#[relationship]` attribute, the only field if unnamed,
/// or the only field in a [`Fields::Named`] with one field, otherwise `Err`.
fn relationship_field<'a>(
fields: &'a Fields,
derive: &'static str,
span: Span,
) -> Result<&'a Field> {
match fields {
Fields::Named(fields) if fields.named.len() == 1 => Ok(fields.named.first().unwrap()),
Fields::Named(fields) => fields.named.iter().find(|field| {
field
.attrs
.iter()
.any(|attr| attr.path().is_ident("relationship"))
}).ok_or(syn::Error::new(
span,
format!("{derive} derive expected named structs with a single field or with a field annotated with #[relationship].")
)),
Fields::Unnamed(fields) => fields
.unnamed
.len()
.eq(&1)
.then(|| fields.unnamed.first())
.flatten()
.ok_or(syn::Error::new(
span,
format!("{derive} derive expected unnamed structs with one field."),
)),
Fields::Unit => Err(syn::Error::new(
span,
format!("{derive} derive expected named or unnamed struct, found unit struct."),
)),
}
}
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/entity/clone_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1291,9 +1291,9 @@ mod tests {
fn recursive_clone() {
let mut world = World::new();
let root = world.spawn_empty().id();
let child1 = world.spawn(ChildOf(root)).id();
let grandchild = world.spawn(ChildOf(child1)).id();
let child2 = world.spawn(ChildOf(root)).id();
let child1 = world.spawn(ChildOf { parent: root }).id();
let grandchild = world.spawn(ChildOf { parent: child1 }).id();
let child2 = world.spawn(ChildOf { parent: root }).id();

let clone_root = world.spawn_empty().id();
EntityCloner::build(&mut world)
Expand Down
Loading

0 comments on commit b688398

Please sign in to comment.