ronantakizawa/codereview-bench huggingface dataset:
Repository: bevyengine/bevy
File: crates/bevy_ui/src/widget/viewport.rs (Rust)

Code before:
            });
        }
    }
}

/// Spawns a new viewport widget with the given `camera` and `target`.
pub fn viewport(camera: Entity, target: Handle<Image>) -> impl Bundle {
    (
        Viewport::new(camera),
        #[cfg(feature = "bevy_ui_picking_backend")]
        PointerId::Custom(Uuid::new_v4()),
        children![(
            ImageNode::new(target),
            Node {
                position_type: PositionType::Absolute,
                top: Val::ZERO,
                bottom: Val::ZERO,
                left: Val::ZERO,
                right: Val::ZERO,
                ..default()
            },
            #[cfg(feature = "bevy_ui_picking_backend")]
            Pickable::IGNORE,
        )],
    )
}

/// Updates the size of the associated render target for viewports when the node size changes.
pub fn update_viewport_render_target_size(
    viewport_query: Query<(&Viewport, &Children)>,
    node_query: Query<&ComputedNode, Changed<ComputedNode>>,
    camera_query: Query<&Camera>,
    mut images: ResMut<Assets<Image>>,
) {
    for (viewport, children) in &viewport_query {
        let Some(computed_node) = children
            .iter()
            .find_map(|child| node_query.get(*child).ok())
        else {
            // Node hasn't been changed, or one wasn't found
            continue;
        };

        let camera = camera_query.get(viewport.camera).unwrap();
        let size = computed_node.size();

        let Some(image_handle) = camera.target.as_image() else {
            continue;
        };
        let size = Extent3d {
            width: u32::max(1, size.x as u32),

Review comment (refactor):
Don't need to make the image a child and instead of `ImageNode` we can use an extraction system, then we can just get image handle from the viewport camera entity.

```suggestion
```

Code after:
        }
    }
}

/// Updates the size of the associated render target for viewports when the node size changes.
pub fn update_viewport_render_target_size(
    viewport_query: Query<
        (&ViewportNode, &ComputedNode),
        Or<(Changed<ComputedNode>, Changed<ViewportNode>)>,
    >,
    camera_query: Query<&Camera>,
    mut images: ResMut<Assets<Image>>,
) {
    for (viewport, computed_node) in &viewport_query {
        let camera = camera_query.get(viewport.camera).unwrap();
        let size = computed_node.size();

        let Some(image_handle) = camera.target.as_image() else {
            continue;
        };
        let size = Extent3d {
            width: u32::max(1, size.x as u32),
            height: u32::max(1, size.y as u32),
            ..default()
        };
        images.get_mut(image_handle).unwrap().resize(size);
    }
}
Repository: bevyengine/bevy
File: crates/bevy_macro_utils/src/shape.rs (Rust)

Code before:
use syn::{
    punctuated::Punctuated, spanned::Spanned, token::Comma, Data, DataEnum, DataUnion, Error,
    Field, Fields,
};

/// Get the fields of a data structure if that structure is a struct with named fields;
/// otherwise, return a compile error that points to the site of the macro invocation.
///
/// `meta` should be the name of the macro calling this function.
pub fn get_struct_fields<'a>(
    data: &'a Data,
    meta: &str,
) -> syn::Result<&'a Punctuated<Field, Comma>> {
    match data {
        Data::Struct(data_struct) => match &data_struct.fields {
            Fields::Named(fields_named) => Ok(&fields_named.named),
            Fields::Unnamed(fields_unnamed) => Ok(&fields_unnamed.unnamed),
            Fields::Unit => Err(Error::new(
                data_struct.fields.span(),
                format!("#[{meta}] only supports structs with more than zero fields"),
            )),
        },
        Data::Enum(DataEnum { enum_token, .. }) => Err(Error::new(
            enum_token.span(),
            format!("#[{meta}] only supports structs, not enums"),
        )),
        Data::Union(DataUnion { union_token, .. }) => Err(Error::new(
            union_token.span(),
            format!("#[{meta}] only supports structs, not unions"),
        )),
    }
}


Review comment (suggestion):
wouldn't change much, but `meta` could be only the name of the trait being derived instead of `derive{trait}`

Code after:
use syn::{
    punctuated::Punctuated, spanned::Spanned, token::Comma, Data, DataEnum, DataUnion, Error,
    Field, Fields,
};

/// Get the fields of a data structure if that structure is a struct with named fields;
/// otherwise, return a compile error that points to the site of the macro invocation.
///
/// `meta` should be the name of the macro calling this function.
pub fn get_struct_fields<'a>(
    data: &'a Data,
    meta: &str,
) -> syn::Result<&'a Punctuated<Field, Comma>> {
    match data {
        Data::Struct(data_struct) => match &data_struct.fields {
            Fields::Named(fields_named) => Ok(&fields_named.named),
            Fields::Unnamed(fields_unnamed) => Ok(&fields_unnamed.unnamed),
            Fields::Unit => Ok(const { &Punctuated::new() }),
        },
        Data::Enum(DataEnum { enum_token, .. }) => Err(Error::new(
            enum_token.span(),
            format!("#[{meta}] only supports structs, not enums"),
        )),
        Data::Union(DataUnion { union_token, .. }) => Err(Error::new(
            union_token.span(),
            format!("#[{meta}] only supports structs, not unions"),
        )),
    }
}
Repository: bevyengine/bevy
File: crates/bevy_macro_utils/src/shape.rs (Rust)

Code before:
use syn::{
    punctuated::Punctuated, spanned::Spanned, token::Comma, Data, DataEnum, DataUnion, Error,
    Field, Fields,
};

/// Get the fields of a data structure if that structure is a struct with named fields;
/// otherwise, return a compile error that points to the site of the macro invocation.
///
/// `meta` should be the name of the macro calling this function.
pub fn get_struct_fields<'a>(
    data: &'a Data,
    meta: &str,
) -> syn::Result<&'a Punctuated<Field, Comma>> {
    match data {
        Data::Struct(data_struct) => match &data_struct.fields {
            Fields::Named(fields_named) => Ok(&fields_named.named),
            Fields::Unnamed(fields_unnamed) => Ok(&fields_unnamed.unnamed),
            Fields::Unit => Err(Error::new(
                data_struct.fields.span(),
                format!("#[{meta}] only supports structs with more than zero fields"),
            )),
        },
        Data::Enum(DataEnum { enum_token, .. }) => Err(Error::new(
            enum_token.span(),
            format!("#[{meta}] only supports structs, not enums"),
        )),
        Data::Union(DataUnion { union_token, .. }) => Err(Error::new(
            union_token.span(),
            format!("#[{meta}] only supports structs, not unions"),
        )),
    }
}


Review comment (suggestion):
I realize this is how the code worked before, but would it make sense to support these derives on unit structs by returning an empty list of fields?  (Although `derive_visit_entities_base` raises a separate error that we'd have to remove to fully support that.)

https://github.com/bevyengine/bevy/blob/edba54adac6c14d6b6f8199bb87c3dcef85a7989/crates/bevy_ecs/macros/src/lib.rs#L231

```suggestion
            Fields::Unit => Ok(const { &Punctuated::new() }),
```

Code after:
use syn::{
    punctuated::Punctuated, spanned::Spanned, token::Comma, Data, DataEnum, DataUnion, Error,
    Field, Fields,
};

/// Get the fields of a data structure if that structure is a struct with named fields;
/// otherwise, return a compile error that points to the site of the macro invocation.
///
/// `meta` should be the name of the macro calling this function.
pub fn get_struct_fields<'a>(
    data: &'a Data,
    meta: &str,
) -> syn::Result<&'a Punctuated<Field, Comma>> {
    match data {
        Data::Struct(data_struct) => match &data_struct.fields {
            Fields::Named(fields_named) => Ok(&fields_named.named),
            Fields::Unnamed(fields_unnamed) => Ok(&fields_unnamed.unnamed),
            Fields::Unit => Ok(const { &Punctuated::new() }),
        },
        Data::Enum(DataEnum { enum_token, .. }) => Err(Error::new(
            enum_token.span(),
            format!("#[{meta}] only supports structs, not enums"),
        )),
        Data::Union(DataUnion { union_token, .. }) => Err(Error::new(
            union_token.span(),
            format!("#[{meta}] only supports structs, not unions"),
        )),
    }
}
Repository: bevyengine/bevy
File: crates/bevy_ecs/src/system/system_registry.rs (Rust)

Code before:

    #[test]
    fn run_system_exclusive_adapters() {
        let mut world = World::new();
        fn system(_: &mut World) {}
        world.run_system_cached(system).unwrap();
        world.run_system_cached(system.pipe(system)).unwrap();
        world.run_system_cached(system.map(|()| {})).unwrap();
    }

    #[test]
    #[should_panic]
    fn removing_system_stack_with_one_shot() {
        let mut world = World::new();
        let _ = world.run_system_cached(|world: &mut World| {
            world.remove_resource::<RunSystemStack>();
        });
    }

    #[test]
    #[should_panic]
    fn removing_system_stack_with_observer() {
        let mut world = World::new();
        world.add_observer(
            |_trigger: Trigger<OnRemove, RegisteredSystem<(), ()>>, world: &mut World| {
                world.remove_resource::<RunSystemStack>();
            },
        );
        let _ = world.run_system_cached(|_world: &mut World| {});
    }
}


Review comment (bug):
I think this is the cause of the miri failure.  It's not supposed to be valid to use an exclusive system as an observer!  I'm pretty sure it's a bug in the observer code that it allows this to be added.  Maybe we need to open an issue for that?

It should work to use `Commands` instead of `&mut World`, though.  

```suggestion
            |_trigger: Trigger<OnRemove, RegisteredSystem<(), ()>>, mut commands: Commands| {
                commands.remove_resource::<RunSystemStack>();
```

Code after:

        let id = world.register_system(with_ref);
        world.run_system_with(id, &2).unwrap();
        assert_eq!(*world.resource::<Counter>(), Counter(2));
    }

    #[test]
    fn system_with_input_mut() {
        #[derive(Event)]
        struct MyEvent {
            cancelled: bool,
        }

        fn post(InMut(event): InMut<MyEvent>, counter: ResMut<Counter>) {
            if counter.0 > 0 {
                event.cancelled = true;
            }
        }

        let mut world = World::new();
        world.insert_resource(Counter(0));
        let post_system = world.register_system(post);

        let mut event = MyEvent { cancelled: false };
        world.run_system_with(post_system, &mut event).unwrap();
        assert!(!event.cancelled);

        world.resource_mut::<Counter>().0 = 1;
        world.run_system_with(post_system, &mut event).unwrap();
        assert!(event.cancelled);
    }

    #[test]
    fn run_system_invalid_params() {
        use crate::system::RegisteredSystemError;
        use alloc::string::ToString;

        struct T;
        impl Resource for T {}
        fn system(_: Res<T>) {}

        let mut world = World::new();
        let id = world.register_system(system);
        // This fails because `T` has not been added to the world yet.
        let result = world.run_system(id);

        assert!(matches!(result, Err(RegisteredSystemError::Failed { .. })));
        let expected = "System returned error: Parameter `Res<T>` failed validation: Resource does not exist\n";
        assert!(result.unwrap_err().to_string().contains(expected));
    }
Repository: bevyengine/bevy
File: crates/bevy_ecs/src/system/system_registry.rs (Rust)

Code before:
use bevy_reflect::Reflect;
use core::{any::TypeId, marker::PhantomData};
use disqualified::ShortName;
use thiserror::Error;

/// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized.
#[derive(Component)]
#[require(SystemIdMarker = SystemIdMarker::typed_system_id_marker::<I, O>(), Internal)]
pub(crate) struct RegisteredSystem<I, O> {
    initialized: bool,
    system: BoxedSystem<I, O>,
}

impl<I, O> RegisteredSystem<I, O> {
    pub fn new(system: BoxedSystem<I, O>) -> Self {
        RegisteredSystem {
            initialized: false,
            system,
        }
    }
}

#[derive(Debug, Clone, Copy)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
#[cfg_attr(feature = "bevy_reflect", reflect(Debug, Default, Clone))]
struct TypeIdAndName(TypeId, &'static str);

impl TypeIdAndName {
    fn new<T: 'static>() -> Self {
        Self(TypeId::of::<T>(), core::any::type_name::<T>())
    }
}

impl Default for TypeIdAndName {
    fn default() -> Self {
        Self(TypeId::of::<()>(), core::any::type_name::<()>())
    }
}

/// Marker [`Component`](bevy_ecs::component::Component) for identifying [`SystemId`] [`Entity`]s.
#[derive(Debug, Default, Clone, Copy, Component)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
#[cfg_attr(feature = "bevy_reflect", reflect(Debug, Default, Clone, Component))]
pub struct SystemIdMarker {
    input_type_id: TypeIdAndName,
    output_type_id: TypeIdAndName,
}

impl SystemIdMarker {
    fn typed_system_id_marker<I: 'static, O: 'static>() -> Self {
        Self {

Review comment (bug):
The `name` here needs to be controlled via the new `debug` feature flag to avoid allocating and leaking strings in production.

Code after:
/// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized.
#[derive(Component)]
#[require(SystemIdMarker = SystemIdMarker::typed_system_id_marker::<I, O>(), Internal)]
pub(crate) struct RegisteredSystem<I, O> {
    initialized: bool,
    system: BoxedSystem<I, O>,
}

impl<I, O> RegisteredSystem<I, O> {
    pub fn new(system: BoxedSystem<I, O>) -> Self {
        RegisteredSystem {
            initialized: false,
            system,
        }
    }
}

#[derive(Debug, Clone)]
struct TypeIdAndName {
    type_id: TypeId,
    name: DebugName,
}

impl TypeIdAndName {
    fn new<T: 'static>() -> Self {
        Self {
            type_id: TypeId::of::<T>(),
            name: DebugName::type_name::<T>(),
        }
    }
}

impl Default for TypeIdAndName {
    fn default() -> Self {
        Self {
            type_id: TypeId::of::<()>(),
            name: DebugName::type_name::<()>(),
        }
    }
}

/// Marker [`Component`](bevy_ecs::component::Component) for identifying [`SystemId`] [`Entity`]s.
#[derive(Debug, Default, Clone, Component)]
pub struct SystemIdMarker {
    input_type_id: TypeIdAndName,
    output_type_id: TypeIdAndName,
}

impl SystemIdMarker {
    fn typed_system_id_marker<I: 'static, O: 'static>() -> Self {
        Self {
Repository: bevyengine/bevy
File: examples/ui/render_ui_to_texture.rs (Rust)

Code before:
use bevy::{
    asset::uuid::Uuid,
    color::palettes::css::{BLUE, GRAY, RED},
    input::ButtonState,
    picking::{
        backend::ray::RayMap,
        pointer::{Location, PointerAction, PointerId, PointerInput},
        PickSet,
    },
    prelude::*,
    render::{
        camera::{ManualTextureViews, RenderTarget},
        render_asset::RenderAssetUsages,
        render_resource::{Extent3d, TextureDimension, TextureFormat, TextureUsages},
    },
    window::{PrimaryWindow, WindowEvent},
};

const CUBE_POINTER_ID: PointerId = PointerId::Custom(Uuid::from_u128(90870987));

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, rotator_system)
        .add_systems(First, drive_diegetic_pointer.in_set(PickSet::Input))
        .run();
}

// Marks the cube, to which the UI texture is applied.
#[derive(Component)]
struct Cube;

fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
    mut images: ResMut<Assets<Image>>,
) {
    let size = Extent3d {
        width: 512,
        height: 512,
        ..default()
    };

    // This is the texture that will be rendered to.
    let mut image = Image::new_fill(
        size,
        TextureDimension::D2,
        &[0, 0, 0, 0],
        TextureFormat::Bgra8UnormSrgb,

Review comment (suggestion):
```suggestion
        .add_systems(First, drive_diegetic_pointer.in_set(PickingSystems::Input))
```

Code after:
use bevy::picking::PickingSystems;
use bevy::{
    asset::{uuid::Uuid, RenderAssetUsages},
    camera::RenderTarget,
    color::palettes::css::{BLUE, GRAY, RED},
    input::ButtonState,
    picking::{
        backend::ray::RayMap,
        pointer::{Location, PointerAction, PointerId, PointerInput},
    },
    prelude::*,
    render::render_resource::{Extent3d, TextureDimension, TextureFormat, TextureUsages},
    window::{PrimaryWindow, WindowEvent},
};

const CUBE_POINTER_ID: PointerId = PointerId::Custom(Uuid::from_u128(90870987));

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, rotator_system)
        .add_systems(First, drive_diegetic_pointer.in_set(PickingSystems::Input))
        .run();
}

// Marks the cube, to which the UI texture is applied.
#[derive(Component)]
struct Cube;

fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
    mut images: ResMut<Assets<Image>>,
) {
    let size = Extent3d {
        width: 512,
        height: 512,
        ..default()
    };

    // This is the texture that will be rendered to.
    let mut image = Image::new_fill(
        size,
        TextureDimension::D2,
        &[0, 0, 0, 0],
        TextureFormat::Bgra8UnormSrgb,
        RenderAssetUsages::default(),
    );
    // You need to set these texture usage flags in order to use the image as a render target
Repository: bevyengine/bevy
File: examples/ui/render_ui_to_texture.rs (Rust)

Code before:
//! Shows how to render UI to a texture. Useful for displaying UI in 3D space.

use std::f32::consts::PI;

use bevy::picking::PickingSystems;
use bevy::{
    asset::uuid::Uuid,
    camera::RenderTarget,
    color::palettes::css::{BLUE, GRAY, RED},
    input::ButtonState,
    picking::{
        backend::ray::RayMap,
        pointer::{Location, PointerAction, PointerId, PointerInput},
    },
    prelude::*,
    render::{
        render_asset::RenderAssetUsages,
        render_resource::{Extent3d, TextureDimension, TextureFormat, TextureUsages},
    },
    window::{PrimaryWindow, WindowEvent},
};

const CUBE_POINTER_ID: PointerId = PointerId::Custom(Uuid::from_u128(90870987));

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, rotator_system)
        .add_systems(First, drive_diegetic_pointer.in_set(PickingSystems::Input))
        .run();
}

Review comment (suggestion):
```suggestion
    asset::{RenderAssetUsages, uuid::Uuid},
```

Code after:
//! Shows how to render UI to a texture. Useful for displaying UI in 3D space.

use std::f32::consts::PI;

use bevy::picking::PickingSystems;
use bevy::{
    asset::{uuid::Uuid, RenderAssetUsages},
    camera::RenderTarget,
    color::palettes::css::{BLUE, GRAY, RED},
    input::ButtonState,
    picking::{
        backend::ray::RayMap,
        pointer::{Location, PointerAction, PointerId, PointerInput},
    },
    prelude::*,
    render::render_resource::{Extent3d, TextureDimension, TextureFormat, TextureUsages},
    window::{PrimaryWindow, WindowEvent},
};

const CUBE_POINTER_ID: PointerId = PointerId::Custom(Uuid::from_u128(90870987));

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, rotator_system)
        .add_systems(First, drive_diegetic_pointer.in_set(PickingSystems::Input))
        .run();
}

// Marks the cube, to which the UI texture is applied.
#[derive(Component)]
Repository: bevyengine/bevy
File: crates/bevy_utils/src/lib.rs (Rust)

Code before:
mod wgpu_wrapper;

mod default;
mod once;
#[cfg(feature = "std")]
mod parallel_queue;

#[doc(hidden)]
pub use once::OnceFlag;

pub use default::default;

pub use wgpu_wrapper::WgpuWrapper;

#[cfg(feature = "std")]
pub use parallel_queue::*;

use core::mem::ManuallyDrop;

#[cfg(feature = "alloc")]
use {
    bevy_platform::{
        collections::HashMap,
        hash::{Hashed, NoOpHash, PassHash},
    },
    core::{any::TypeId, hash::Hash},
};

/// A [`HashMap`] pre-configured to use [`Hashed`] keys and [`PassHash`] passthrough hashing.
/// Iteration order only depends on the order of insertions and deletions.
#[cfg(feature = "alloc")]
pub type PreHashMap<K, V> = HashMap<Hashed<K>, V, PassHash>;

/// Extension methods intended to add functionality to [`PreHashMap`].
#[cfg(feature = "alloc")]
pub trait PreHashMapExt<K, V> {
    /// Tries to get or insert the value for the given `key` using the pre-computed hash first.
    /// If the [`PreHashMap`] does not already contain the `key`, it will clone it and insert
    /// the value returned by `func`.
    fn get_or_insert_with<F: FnOnce() -> V>(&mut self, key: &Hashed<K>, func: F) -> &mut V;
}

#[cfg(feature = "alloc")]
impl<K: Hash + Eq + PartialEq + Clone, V> PreHashMapExt<K, V> for PreHashMap<K, V> {
    #[inline]
    fn get_or_insert_with<F: FnOnce() -> V>(&mut self, key: &Hashed<K>, func: F) -> &mut V {
        use bevy_platform::collections::hash_map::RawEntryMut;
        let entry = self
            .raw_entry_mut()
            .from_key_hashed_nocheck(key.hash(), key);
        match entry {

Review comment (suggestion):
```suggestion
#[cfg(feature = "std")]
mod wgpu_wrapper;
```

Code after:
    extern crate std;
}

cfg::alloc! {
    extern crate alloc;

    mod map;
    pub use map::*;
}

cfg::parallel! {
    mod parallel_queue;
    pub use parallel_queue::*;
}

/// The utilities prelude.
///
/// This includes the most common types in this crate, re-exported for your convenience.
pub mod prelude {
    pub use crate::default;
}

pub mod synccell;
pub mod syncunsafecell;
#[cfg(feature = "wgpu_wrapper")]
mod wgpu_wrapper;

mod default;
mod once;

#[doc(hidden)]
pub use once::OnceFlag;

pub use default::default;

#[cfg(feature = "wgpu_wrapper")]
pub use wgpu_wrapper::WgpuWrapper;

use core::mem::ManuallyDrop;

/// A type which calls a function when dropped.
/// This can be used to ensure that cleanup code is run even in case of a panic.
///
/// Note that this only works for panics that [unwind](https://doc.rust-lang.org/nomicon/unwinding.html)
/// -- any code within `OnDrop` will be skipped if a panic does not unwind.
/// In most cases, this will just work.
///
/// # Examples
///
/// ```
/// # use bevy_utils::OnDrop;