ronantakizawa/codereview-bench huggingface dataset:
Repository: bevyengine/bevy
File: examples/ui/image_node_resizing.rs (Rust)

Code before:

// Text data for easy modification
#[derive(Debug, Component)]
struct TextData {
    height: f32,
    width: f32,
}

#[derive(Debug)]
enum Direction {
    Height,
    Width,
}

#[derive(Debug, EntityEvent)]
struct TextUpdate {
    entity: Entity,
    direction: Direction,
    change: f32,
}

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    let image_handle = asset_server.load("branding/icon.png");
    let full_text = format!(
        "{}height : {}%, width : {}%",
        TEXT_PREFIX, IMAGE_GROUP_BOX_INIT_HEIGHT, IMAGE_GROUP_BOX_INIT_WIDTH, 
    );

    commands.spawn(Camera2d);

    let container = commands
        .spawn((
            Node {
                display: Display::Grid,
                width: percent(100),
                height: percent(100),
                grid_template_rows: vec![GridTrack::min_content(), GridTrack::flex(1.0)],
                ..default()
            },
            BackgroundColor(Color::WHITE),
        ))
        .id();

    // Keyboard Text
    commands
        .spawn((
            TextData {
                height: IMAGE_GROUP_BOX_INIT_HEIGHT,
                width: IMAGE_GROUP_BOX_INIT_WIDTH,
            },
            Text::new(full_text),

Review comment (suggestion):
Oops, cargo fmt is complaining about the trailing space
```suggestion
        TEXT_PREFIX, IMAGE_GROUP_BOX_INIT_HEIGHT, IMAGE_GROUP_BOX_INIT_WIDTH,
```

Code after:

// Text data for easy modification
#[derive(Debug, Component)]
struct TextData {
    height: f32,
    width: f32,
}

#[derive(Debug)]
enum Direction {
    Height,
    Width,
}

#[derive(Debug, EntityEvent)]
struct TextUpdate {
    entity: Entity,
    direction: Direction,
    change: f32,
}

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    let image_handle = asset_server.load("branding/icon.png");
    let full_text = format!(
        "{}height : {}%, width : {}%",
        TEXT_PREFIX, IMAGE_GROUP_BOX_INIT_HEIGHT, IMAGE_GROUP_BOX_INIT_WIDTH,
    );

    commands.spawn(Camera2d);

    let container = commands
        .spawn((
            Node {
                display: Display::Grid,
                width: percent(100),
                height: percent(100),
                grid_template_rows: vec![GridTrack::min_content(), GridTrack::flex(1.0)],
                ..default()
            },
            BackgroundColor(Color::WHITE),
        ))
        .id();

    // Keyboard Text
    commands
        .spawn((
            TextData {
                height: IMAGE_GROUP_BOX_INIT_HEIGHT,
                width: IMAGE_GROUP_BOX_INIT_WIDTH,
            },
            Text::new(full_text),
Repository: bevyengine/bevy
File: crates/bevy_input_focus/src/directional_navigation.rs (Rust)

Code before:
        let mut nav_map = DirectionalNavigationMap::default();
        let config = AutoNavigationConfig::default();

        let node_a = Entity::from_bits(1);
        let node_b = Entity::from_bits(2);
        let node_c = Entity::from_bits(3);

        // Manually set a block from A to B
        // A should NOT be able to nav East to B
        // but SHOULD be able to nav South to C
        nav_map.block_edge(node_a, CompassOctant::East);

        let nodes = vec![
            FocusableArea {
                entity: node_a,
                position: Vec2::new(0.0, 0.0),
                size: Vec2::new(50.0, 50.0),
            },
            FocusableArea {
                entity: node_b,
                position: Vec2::new(50.0, 0.0),
                size: Vec2::new(50.0, 50.0),
            },
            FocusableArea {
                entity: node_c,
                position: Vec2::new(0.0, -50.0),
                size: Vec2::new(50.0, 50.0),
            },
        ];

        auto_generate_navigation_edges(&mut nav_map, &nodes, &config);

        // The manual edge should be preserved, even though B is closer
        assert_eq!(
            nav_map.get_neighbor(node_a, CompassOctant::East),
            NavNeighbor::Blocked
        );
        // But automatic edges should still be populated
        assert_eq!(
            nav_map.get_neighbor(node_a, CompassOctant::South),
            NavNeighbor::Set(node_c)
        );
    }
}


Review comment (suggestion):
```suggestion
                position: Vec2::new(0.0, 50.0),
```
to be truly south

Code after:
        let mut nav_map = DirectionalNavigationMap::default();
        let config = AutoNavigationConfig::default();

        let node_a = Entity::from_bits(1);
        let node_b = Entity::from_bits(2);
        let node_c = Entity::from_bits(3);

        // Manually set a block from A to B
        // A should NOT be able to nav East to B
        // but SHOULD be able to nav South to C
        nav_map.block_edge(node_a, CompassOctant::East);

        let nodes = vec![
            FocusableArea {
                entity: node_a,
                position: Vec2::new(0.0, 0.0),
                size: Vec2::new(50.0, 50.0),
            },
            FocusableArea {
                entity: node_b,
                position: Vec2::new(50.0, 0.0),
                size: Vec2::new(50.0, 50.0),
            },
            FocusableArea {
                entity: node_c,
                position: Vec2::new(0.0, 50.0),
                size: Vec2::new(50.0, 50.0),
            },
        ];

        auto_generate_navigation_edges(&mut nav_map, &nodes, &config);

        // The manual edge should be preserved, even though B is closer
        assert_eq!(
            nav_map.get_neighbor(node_a, CompassOctant::East),
            NavNeighbor::Blocked
        );
        // But automatic edges should still be populated
        assert_eq!(
            nav_map.get_neighbor(node_a, CompassOctant::South),
            NavNeighbor::Set(node_c)
        );
    }
}
Repository: bevyengine/bevy
File: crates/bevy_reflect/src/structs.rs (Rust)

Code before:
            let mut result = false;
            if i == index {
                self.field_indices
                    .remove(n)
                    .expect("Invalid name for `field_indices.remove(name)`");
                result = true;
            } else if i > index {
                *self
                    .field_indices
                    .get_mut(n)
                    .expect("Invalid name for `field_indices.get_mut(name)`") -= 1;
            }
            i += 1;
            result
        });

        let name = extract
            .nth(0)
            .expect("Invalid index for `extract.nth(index)`");
        extract.for_each(drop); // Fully evaluate the rest of the iterator, so we don't short-circuit the extract.

        Some((name, self.fields.remove(index)))
    }

    /// Removes a field with the predicate `f`, that short-circuits.
    pub fn remove_with<F>(
        &mut self,
        mut f: F,
    ) -> Option<(Cow<'static, str>, Box<dyn PartialReflect>)>
    where
        F: FnMut((&str, &dyn PartialReflect)) -> bool,
    {
        if let Some(index) = self
            .field_names
            .iter()
            .zip(self.fields.iter())
            .position(|(name, field)| f((name.as_ref(), field.as_ref())))
        {
            self.remove_at(index)
        } else {
            None
        }
    }

    /// Removes a field by `name`.
    pub fn remove_by_name(
        &mut self,
        name: &str,
    ) -> Option<(Cow<'static, str>, Box<dyn PartialReflect>)> {
        if let Some(index) = self.index_of_name(name) {
            self.remove_at(index)

Review comment (suggestion):
Nit: Maybe `remove_if` or something? `remove_with` is okay but I feel it could be a bit more explicit

Code after:
        F: FnMut((&str, &dyn PartialReflect)) -> bool,
    {
        if let Some(index) = self
            .field_names
            .iter()
            .zip(self.fields.iter())
            .position(|(name, field)| f((name.as_ref(), field.as_ref())))
        {
            self.remove_at(index)
        } else {
            None
        }
    }

    /// Removes a field by `name`.
    pub fn remove_by_name(
        &mut self,
        name: &str,
    ) -> Option<(Cow<'static, str>, Box<dyn PartialReflect>)> {
        if let Some(index) = self.index_of(name) {
            self.remove_at(index)
        } else {
            None
        }
    }

    /// Gets the index of the field with the given name.
    pub fn index_of(&self, name: &str) -> Option<usize> {
        self.field_indices.get(name).copied()
    }
}

impl Struct for DynamicStruct {
    #[inline]
    fn field(&self, name: &str) -> Option<&dyn PartialReflect> {
        self.field_indices
            .get(name)
            .map(|index| &*self.fields[*index])
    }

    #[inline]
    fn field_mut(&mut self, name: &str) -> Option<&mut dyn PartialReflect> {
        if let Some(index) = self.field_indices.get(name) {
            Some(&mut *self.fields[*index])
        } else {
            None
        }
    }

    #[inline]
    fn field_at(&self, index: usize) -> Option<&dyn PartialReflect> {
Repository: bevyengine/bevy
File: crates/bevy_render/src/extract_plugin.rs (Rust)

Code before:
use core::ops::{Deref, DerefMut};

use crate::{
    sync_world::{despawn_temporary_render_entities, entity_sync_system, SyncWorldPlugin},
    Render, RenderApp, RenderSystems,
};
use bevy_app::{App, Plugin, SubApp};
use bevy_ecs::{
    resource::Resource,
    schedule::{IntoScheduleConfigs, Schedule, ScheduleBuildSettings, ScheduleLabel, Schedules},
    world::{Mut, World},
};
use bevy_utils::default;

/// Plugin that sets up the [`RenderApp`] and handles extracting data from the
/// main world to the render world.
#[derive(Default)]
pub struct ExtractPlugin {
    /// Function that gets run at the beginning of each extraction.
    ///
    /// Gets the main world and render world as arguments (in that order).
    pub extract_callback: Option<fn(&mut World, &mut World)>,
}

impl Plugin for ExtractPlugin {
    fn build(&self, app: &mut App) {
        app.add_plugins(SyncWorldPlugin);
        app.init_resource::<ScratchMainWorld>();

        let mut render_app = SubApp::new();

Review comment (suggestion):
```suggestion
use bevy_app::{App, Plugin, SubApp};
use bevy_derive::{Deref, DerefMut};
```

Code after:
use crate::{
    sync_world::{despawn_temporary_render_entities, entity_sync_system, SyncWorldPlugin},
    Render, RenderApp, RenderSystems,
};
use bevy_app::{App, Plugin, SubApp};
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{
    resource::Resource,
    schedule::{IntoScheduleConfigs, Schedule, ScheduleBuildSettings, ScheduleLabel, Schedules},
    world::{Mut, World},
};
use bevy_utils::default;

/// Plugin that sets up the [`RenderApp`] and handles extracting data from the
/// main world to the render world.
pub struct ExtractPlugin {
    /// Function that gets run at the beginning of each extraction.
    ///
    /// Gets the main world and render world as arguments (in that order).
    pub pre_extract: fn(&mut World, &mut World),
}

impl Default for ExtractPlugin {
    fn default() -> Self {
        Self {
            pre_extract: |_, _| {},
        }
    }
}
Repository: bevyengine/bevy
File: crates/bevy_gizmos/src/lib.rs (Rust)

Code before:
        &self.buffer
    }
}

impl GizmoAsset {
    /// Create a new [`GizmoAsset`].
    pub fn new() -> Self {
        GizmoAsset {
            buffer: GizmoBuffer::default(),
            config_ty: TypeId::of::<ErasedGizmoConfigGroup>(),
        }
    }

    /// The type of the gizmo's configuration group.
    pub fn config_typeid(&self) -> TypeId {
        self.config_ty
    }
}

impl Default for GizmoAsset {
    fn default() -> Self {
        GizmoAsset::new()
    }
}

/// Used by aabb and frustum gizmos for line color
pub(crate) fn color_from_entity(entity: Entity) -> Color {
    Oklcha::sequential_dispersed(entity.index_u32()).into()
}


Review comment (suggestion):
```suggestion
/// Generates a random, well-dispersed color seeded by the provided `Entity`.
```

Code after:
        &self.buffer
    }
}

impl GizmoAsset {
    /// Create a new [`GizmoAsset`].
    pub fn new() -> Self {
        GizmoAsset {
            buffer: GizmoBuffer::default(),
            config_ty: TypeId::of::<ErasedGizmoConfigGroup>(),
        }
    }

    /// The type of the gizmo's configuration group.
    pub fn config_typeid(&self) -> TypeId {
        self.config_ty
    }
}

impl Default for GizmoAsset {
    fn default() -> Self {
        GizmoAsset::new()
    }
}

/// Generates a random, well-dispersed color seeded by the provided `Entity`.
pub fn color_from_entity(entity: Entity) -> Color {
    Oklcha::sequential_dispersed(entity.index_u32()).into()
}
Repository: bevyengine/bevy
File: crates/bevy_gizmos/src/lib.rs (Rust)

Code before:
    }
}

impl GizmoAsset {
    /// Create a new [`GizmoAsset`].
    pub fn new() -> Self {
        GizmoAsset {
            buffer: GizmoBuffer::default(),
            config_ty: TypeId::of::<ErasedGizmoConfigGroup>(),
        }
    }

    /// The type of the gizmo's configuration group.
    pub fn config_typeid(&self) -> TypeId {
        self.config_ty
    }
}

impl Default for GizmoAsset {
    fn default() -> Self {
        GizmoAsset::new()
    }
}

/// Used by aabb and frustum gizmos for line color
pub(crate) fn color_from_entity(entity: Entity) -> Color {
    Oklcha::sequential_dispersed(entity.index_u32()).into()
}


Review comment (refactor):
```suggestion
pub fn color_from_entity(entity: Entity) -> Color {
```

This is a nice little helper; might as well make it pub for users to enjoy.

Code after:
    }
}

impl GizmoAsset {
    /// Create a new [`GizmoAsset`].
    pub fn new() -> Self {
        GizmoAsset {
            buffer: GizmoBuffer::default(),
            config_ty: TypeId::of::<ErasedGizmoConfigGroup>(),
        }
    }

    /// The type of the gizmo's configuration group.
    pub fn config_typeid(&self) -> TypeId {
        self.config_ty
    }
}

impl Default for GizmoAsset {
    fn default() -> Self {
        GizmoAsset::new()
    }
}

/// Generates a random, well-dispersed color seeded by the provided `Entity`.
pub fn color_from_entity(entity: Entity) -> Color {
    Oklcha::sequential_dispersed(entity.index_u32()).into()
}
Repository: bevyengine/bevy
File: crates/bevy_pbr/src/prepass/mod.rs (Rust)

Code before:
    type Param = SRes<PrepassViewBindGroup>;
    type ViewQuery = ();
    type ItemQuery = ();

    #[inline]
    fn render<'w>(
        _item: &P,
        _view: (),
        _entity: Option<()>,
        prepass_view_bind_group: SystemParamItem<'w, '_, Self::Param>,
        pass: &mut TrackedRenderPass<'w>,
    ) -> RenderCommandResult {
        let prepass_view_bind_group = prepass_view_bind_group.into_inner();
        pass.set_bind_group(I, &prepass_view_bind_group.empty_bind_group, &[]);
        RenderCommandResult::Success
    }
}

pub type DrawPrepass = (
    SetItemPipeline,
    SetPrepassViewBindGroup<0>,
    SetPrepassViewEmptyBindGroup<1>,
    SetMeshBindGroup<2>,
    SetMaterialBindGroup<3>,
    DrawMesh,
);

pub type DrawDepthOnlyPrepass = (
    SetItemPipeline,
    SetPrepassViewBindGroup<0>,
    SetPrepassViewEmptyBindGroup<1>,
    SetMeshBindGroup<2>,
    SetPrepassEmptyMaterialBindGroup<3>,
    DrawMesh,
);


Review comment (suggestion):
I wonder if we could have a more generalized `SetEmptyBindGroup`, render command. I also needed pretty much the same thing recently.

Code after:
    fn render<'w>(
        _item: &P,
        _view: (),
        _entity: Option<()>,
        prepass_view_bind_group: SystemParamItem<'w, '_, Self::Param>,
        pass: &mut TrackedRenderPass<'w>,
    ) -> RenderCommandResult {
        let prepass_view_bind_group = prepass_view_bind_group.into_inner();
        pass.set_bind_group(I, &prepass_view_bind_group.empty_bind_group, &[]);
        RenderCommandResult::Success
    }
}

pub type DrawPrepass = (
    SetItemPipeline,
    SetPrepassViewBindGroup<0>,
    SetPrepassViewEmptyBindGroup<1>,
    SetMeshBindGroup<2>,
    SetMaterialBindGroup<3>,
    DrawMesh,
);

pub type DrawDepthOnlyPrepass = (
    SetItemPipeline,
    SetPrepassViewBindGroup<0>,
    SetPrepassViewEmptyBindGroup<1>,
    SetMeshBindGroup<2>,
    SetPrepassEmptyMaterialBindGroup<3>,
    DrawMesh,
);
Repository: bevyengine/bevy
File: examples/ecs/delayed_commands.rs (Rust)

Code before:
//! This example demonstrates an effect that's easy to compose with delayed commands.

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, spawn)
        .add_observer(click)
        .run();
}

#[derive(Component)]
struct BlinkySquare;

const SQUARE_SIZE: Vec2 = Vec2::splat(45.0);

fn spawn(mut commands: Commands) {
    commands.spawn(Camera2d);
    for x in -5..=5 {
        for y in -5..=5 {
            commands.spawn((
                BlinkySquare,
                Transform::from_xyz(x as f32 * 50.0, y as f32 * 50.0, 0.0),
                Sprite::from_color(Color::BLACK, SQUARE_SIZE),
            ));

Review comment (suggestion):
```suggestion
//! This example demonstrates how to send commands which will take effect after a period of time.
//!
//! We've chosen to demonstrate this effect through the creation of a grid of clickable,
//! with "ripples" created when you click.
```

Code after:
//! This example demonstrates how to send commands which will take effect after a period of time.
//!
//! We've chosen to demonstrate this effect through the creation of a grid of clickable,
//! with "ripples" created when you click.

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, spawn)
        .add_observer(click)
        .run();
}

#[derive(Component)]
struct BlinkySquare;

const SQUARE_SIZE: Vec2 = Vec2::splat(45.0);

fn spawn(mut commands: Commands) {
    commands.spawn(Camera2d);
    for x in -5..=5 {
        for y in -5..=5 {
            commands.spawn((
                BlinkySquare,
Repository: bevyengine/bevy
File: examples/asset/generated_assets.rs (Rust)

Code before:
//! Demonstrates how to use assets generated at runtime.

use std::{thread::sleep, time::Duration};

use bevy::{
    asset::RenderAssetUsages,
    camera_controller::free_camera::{FreeCamera, FreeCameraPlugin},
    mesh::{Indices, PrimitiveTopology},
    prelude::*,
    render::render_resource::{Extent3d, TextureDimension, TextureFormat},
    time::common_conditions::once_after_delay,
};
use rand::{rngs::StdRng, RngExt, SeedableRng};

fn main() {
    App::new()
        .add_plugins((DefaultPlugins, FreeCameraPlugin))
        .add_systems(Startup, setup)
        .add_systems(
            Update,
            generate_image.run_if(once_after_delay(Duration::from_secs(4))),
        )
        .run();
}

fn setup(

Review comment (suggestion):
```suggestion
//! Shows how to add assets generated at runtime
```

Nit: would prefer this to be consistent with the example description used elsewhere.

Code after:
//! Shows how to generate and store assets at runtime.

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, generate_mesh_system.run_if(run_once))
        .run();
}

fn setup(
    mut commands: Commands,
    asset_server: Res<AssetServer>,
    mut materials: ResMut<Assets<StandardMaterial>>,
    meshes: Res<Assets<Mesh>>,
) {
    commands.spawn((Camera3d::default(), Transform::from_xyz(0.0, 0.0, 5.0)));

    commands.spawn((
        DirectionalLight::default(),
        Transform::default().looking_to(Dir3::new(Vec3::new(-1.0, -1.0, -1.0)).unwrap(), Dir3::Y),
    ));

    // The simplest way to generate an asset is to add it directly to the `Assets`.
Repository: bevyengine/bevy
File: examples/asset/generated_assets.rs (Rust)

Code before:
//! Demonstrates how to use assets generated at runtime.

use std::{thread::sleep, time::Duration};

use bevy::{
    asset::RenderAssetUsages,
    camera_controller::free_camera::{FreeCamera, FreeCameraPlugin},
    mesh::{Indices, PrimitiveTopology},
    prelude::*,
    render::render_resource::{Extent3d, TextureDimension, TextureFormat},
    time::common_conditions::once_after_delay,
};
use rand::{rngs::StdRng, RngExt, SeedableRng};

fn main() {
    App::new()
        .add_plugins((DefaultPlugins, FreeCameraPlugin))
        .add_systems(Startup, setup)
        .add_systems(
            Update,
            generate_image.run_if(once_after_delay(Duration::from_secs(4))),
        )
        .run();
}

fn setup(

Review comment (suggestion):
```suggestion
//! Demonstrates how to generate and store assets at runtime.
```

Code after:
//! Shows how to generate and store assets at runtime.

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, generate_mesh_system.run_if(run_once))
        .run();
}

fn setup(
    mut commands: Commands,
    asset_server: Res<AssetServer>,
    mut materials: ResMut<Assets<StandardMaterial>>,
    meshes: Res<Assets<Mesh>>,
) {
    commands.spawn((Camera3d::default(), Transform::from_xyz(0.0, 0.0, 5.0)));

    commands.spawn((
        DirectionalLight::default(),
        Transform::default().looking_to(Dir3::new(Vec3::new(-1.0, -1.0, -1.0)).unwrap(), Dir3::Y),
    ));

    // The simplest way to generate an asset is to add it directly to the `Assets`.