ronantakizawa/codereview-bench huggingface dataset:
Repository: bevyengine/bevy
File: crates/bevy_render/src/render_resource/uniform_buffer.rs (Rust)

Code before:
}

/// A writer that can be used to directly write elements into the target buffer.
///
/// For more information, see [`DynamicUniformBuffer::get_writer`].
pub struct DynamicUniformBufferWriter<T> {
    buffer: encase::DynamicUniformBuffer<QueueWriteBufferViewWrapper>,
    _marker: PhantomData<fn() -> T>,
}

impl<T: ShaderType + WriteInto> DynamicUniformBufferWriter<T> {
    pub fn write(&mut self, value: &T) -> u32 {
        self.buffer.write(value).unwrap() as u32
    }
}

/// A wrapper to work around the orphan rule so that [`wgpu::QueueWriteBufferView`] can implement
/// [`BufferMut`].
struct QueueWriteBufferViewWrapper {
    buffer_view: wgpu::QueueWriteBufferView,
    // Must be kept separately and cannot be retrieved from buffer_view, as the read-only access will
    // invoke a panic.
    capacity: usize,
}

impl<'a> BufferMut for QueueWriteBufferViewWrapper {
    #[inline]
    fn capacity(&self) -> usize {
        self.capacity
    }

    #[inline]
    fn write<const N: usize>(&mut self, offset: usize, val: &[u8; N]) {
        self.buffer_view.write(offset, val);
    }

    #[inline]
    fn write_slice(&mut self, offset: usize, val: &[u8]) {
        self.buffer_view.write_slice(offset, val);
    }
}

impl<'a, T: ShaderType + WriteInto> IntoBinding<'a> for &'a DynamicUniformBuffer<T> {
    #[inline]
    fn into_binding(self) -> BindingResource<'a> {
        self.binding().unwrap()
    }
}


Review comment (suggestion):
```suggestion
impl BufferMut for QueueWriteBufferViewWrapper {
```
it looks like this lifetime was missed in the cleanup

Code after:
}

/// A writer that can be used to directly write elements into the target buffer.
///
/// For more information, see [`DynamicUniformBuffer::get_writer`].
pub struct DynamicUniformBufferWriter<T> {
    buffer: encase::DynamicUniformBuffer<QueueWriteBufferViewWrapper>,
    _marker: PhantomData<fn() -> T>,
}

impl<T: ShaderType + WriteInto> DynamicUniformBufferWriter<T> {
    pub fn write(&mut self, value: &T) -> u32 {
        self.buffer.write(value).unwrap() as u32
    }
}

/// A wrapper to work around the orphan rule so that [`wgpu::QueueWriteBufferView`] can implement
/// [`BufferMut`].
struct QueueWriteBufferViewWrapper {
    buffer_view: wgpu::QueueWriteBufferView,
    // Must be kept separately and cannot be retrieved from buffer_view, as the read-only access will
    // invoke a panic.
    capacity: usize,
}

impl BufferMut for QueueWriteBufferViewWrapper {
    #[inline]
    fn capacity(&self) -> usize {
        self.capacity
    }

    #[inline]
    fn write<const N: usize>(&mut self, offset: usize, val: &[u8; N]) {
        self.buffer_view.write(offset, val);
    }

    #[inline]
    fn write_slice(&mut self, offset: usize, val: &[u8]) {
        self.buffer_view.write_slice(offset, val);
    }
}

impl<'a, T: ShaderType + WriteInto> IntoBinding<'a> for &'a DynamicUniformBuffer<T> {
    #[inline]
    fn into_binding(self) -> BindingResource<'a> {
        self.binding().unwrap()
    }
}
Repository: bevyengine/bevy
File: crates/bevy_ecs/src/component/mod.rs (Rust)

Code before:
///     // ...
/// }
/// ```
///
/// This also supports function calls that yield closures
///
/// ```
/// # use bevy_ecs::component::Component;
/// # use bevy_ecs::lifecycle::HookContext;
/// # use bevy_ecs::world::DeferredWorld;
/// #
/// #[derive(Component)]
/// #[component(on_add = my_msg_hook("hello"))]
/// #[component(on_despawn = my_msg_hook("yoink"))]
/// struct ComponentA;
///
/// // a hook closure generating function
/// fn my_msg_hook(message: &'static str) -> impl Fn(DeferredWorld, HookContext) {
///     move |_world, _ctx| {
///         println!("{message}");
///     }
/// }
///
/// ```
///
/// Hook's function path can be ignored if it is `Self::on_add`, `Self::on_insert` etc.
/// ```
/// # use bevy_ecs::lifecycle::HookContext;
/// # use bevy_ecs::prelude::*;
/// # use bevy_ecs::world::DeferredWorld;
/// #
/// #[derive(Component, Debug)]
/// #[component(on_add)]
/// struct DoubleOnSpawn(usize);
///
/// impl DoubleOnSpawn {
///     fn on_add(mut world: DeferredWorld, context: HookContext) {
///         let mut entity = world.get_mut::<Self>(context.entity).unwrap();
///         entity.0 *= 2;
///     }
/// }
/// #
/// # let mut world = World::new();
/// # let entity = world.spawn(DoubleOnSpawn(2));
/// # assert_eq!(entity.get::<DoubleOnSpawn>().unwrap().0, 4);
/// ```
///
/// # Setting the clone behavior
///
/// You can specify how the [`Component`] is cloned when deriving it.
///

Review comment (suggestion):
Same here:

```suggestion
/// Hook's function path can be elided if it is `Self::on_add`, `Self::on_insert` etc.
```

Code after:
///     // ...
/// }
/// ```
///
/// This also supports function calls that yield closures
///
/// ```
/// # use bevy_ecs::component::Component;
/// # use bevy_ecs::lifecycle::HookContext;
/// # use bevy_ecs::world::DeferredWorld;
/// #
/// #[derive(Component)]
/// #[component(on_add = my_msg_hook("hello"))]
/// #[component(on_despawn = my_msg_hook("yoink"))]
/// struct ComponentA;
///
/// // a hook closure generating function
/// fn my_msg_hook(message: &'static str) -> impl Fn(DeferredWorld, HookContext) {
///     move |_world, _ctx| {
///         println!("{message}");
///     }
/// }
///
/// ```
///
/// A hook's function path can be elided if it is `Self::on_add`, `Self::on_insert` etc.
/// ```
/// # use bevy_ecs::lifecycle::HookContext;
/// # use bevy_ecs::prelude::*;
/// # use bevy_ecs::world::DeferredWorld;
/// #
/// #[derive(Component, Debug)]
/// #[component(on_add)]
/// struct DoubleOnSpawn(usize);
///
/// impl DoubleOnSpawn {
///     fn on_add(mut world: DeferredWorld, context: HookContext) {
///         let mut entity = world.get_mut::<Self>(context.entity).unwrap();
///         entity.0 *= 2;
///     }
/// }
/// #
/// # let mut world = World::new();
/// # let entity = world.spawn(DoubleOnSpawn(2));
/// # assert_eq!(entity.get::<DoubleOnSpawn>().unwrap().0, 4);
/// ```
///
/// # Setting the clone behavior
///
/// You can specify how the [`Component`] is cloned when deriving it.
///
Repository: bevyengine/bevy
File: crates/bevy_feathers/src/cursor.rs (Rust)

Code before:
    let cursor = (**r_override_cursor)
        .as_ref()
        .unwrap_or_else(|| {
            hover_map
                .and_then(|hover_map| match hover_map.get(&PointerId::Mouse) {
                    Some(hover_set) => hover_set.keys().find_map(|entity| {
                        cursor_query.get(*entity).ok().or_else(|| {
                            parent_query
                                .iter_ancestors(*entity)
                                .find_map(|e| cursor_query.get(e).ok())
                        })
                    }),
                    None => None,
                })
                .unwrap_or_else(|| &r_default_cursor)
        })
        .to_cursor_icon();

    for (entity, prev_cursor) in q_windows.iter() {
        if let Some(prev_cursor) = prev_cursor
            && cursor == *prev_cursor
        {
            continue;
        }
        commands.entity(entity).insert(cursor.clone());
    }
}

/// Plugin that supports automatically changing the cursor based on the hovered entity.
pub struct CursorIconPlugin;

impl Plugin for CursorIconPlugin {
    fn build(&self, app: &mut App) {
        if app.world().get_resource::<DefaultCursor>().is_none() {
            app.init_resource::<DefaultCursor>();
            app.init_resource::<OverrideCursor>();
        }
        app.add_systems(PreUpdate, update_cursor.in_set(PickingSystems::Last));
    }
}


Review comment (suggestion):
These changes can now be reverted after latest fixes. No need to unnecessary convert `to_cursor_icon` and to clone it one more time.

Code after:
    let cursor = r_override_cursor.0.as_ref().unwrap_or_else(|| {
        hover_map
            .and_then(|hover_map| match hover_map.get(&PointerId::Mouse) {
                Some(hover_set) => hover_set.keys().find_map(|entity| {
                    cursor_query.get(*entity).ok().or_else(|| {
                        parent_query
                            .iter_ancestors(*entity)
                            .find_map(|e| cursor_query.get(e).ok())
                    })
                }),
                None => None,
            })
            .unwrap_or(&r_default_cursor)
    });

    for (entity, prev_cursor) in q_windows.iter() {
        if let Some(prev_cursor) = prev_cursor
            && cursor.eq_cursor_icon(prev_cursor)
        {
            continue;
        }
        commands.entity(entity).insert(cursor.to_cursor_icon());
    }
}

/// Plugin that supports automatically changing the cursor based on the hovered entity.
pub struct CursorIconPlugin;

impl Plugin for CursorIconPlugin {
    fn build(&self, app: &mut App) {
        if app.world().get_resource::<DefaultCursor>().is_none() {
            app.init_resource::<DefaultCursor>();
            app.init_resource::<OverrideCursor>();
        }
        app.add_systems(PreUpdate, update_cursor.in_set(PickingSystems::Last));
    }
}
Repository: bevyengine/bevy
File: examples/ecs/state_scoped.rs (Rust)

Code before:
//! Shows how to spawn entities that are automatically despawned either when
//! entering or exiting specific game states.
//!
//! This pattern is useful for managing menus, levels, or other state-specific
//! content that should only exist during certain states.
//!
//! If the entity was already despawned then no error will be logged
//! This means that you don't have to worry about duplicating in a hierarchy

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .init_state::<GameState>()
        .add_systems(Startup, setup_camera)
        .add_systems(OnEnter(GameState::A), on_a_enter)
        .add_systems(OnEnter(GameState::B), on_b_enter)
        .add_systems(OnExit(GameState::A), on_a_exit)
        .add_systems(OnExit(GameState::B), on_b_exit)
        .add_systems(Update, toggle)
        .insert_resource(TickTock(Timer::from_seconds(1.0, TimerMode::Repeating)))
        .run();
}

#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Default, States)]
enum GameState {
    #[default]
    A,
    B,
}

#[derive(Resource)]

Review comment (refactor):
```suggestion
//! If the entity was already despawned then no error will be logged. This means
//! that you don't have to worry about duplicate [`DespawnOnExit`] and
//! [`DespawnOnEnter`] components deep in your hierarchy.
```

Code after:
//! Shows how to spawn entities that are automatically despawned either when
//! entering or exiting specific game states.
//!
//! This pattern is useful for managing menus, levels, or other state-specific
//! content that should only exist during certain states.
//!
//! If the entity was already despawned then no error will be logged. This means
//! that you don't have to worry about duplicate [`DespawnOnExit`] and
//! [`DespawnOnEnter`] components deep in your hierarchy.

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .init_state::<GameState>()
        .add_systems(Startup, setup_camera)
        .add_systems(OnEnter(GameState::A), on_a_enter)
        .add_systems(OnEnter(GameState::B), on_b_enter)
        .add_systems(OnExit(GameState::A), on_a_exit)
        .add_systems(OnExit(GameState::B), on_b_exit)
        .add_systems(Update, toggle)
        .insert_resource(TickTock(Timer::from_seconds(1.0, TimerMode::Repeating)))
        .run();
}

#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Default, States)]
enum GameState {
    #[default]
    A,
    B,
}
Repository: bevyengine/bevy
File: crates/bevy_asset/src/meta.rs (Rust)

Code before:
                core::any::type_name::<S>(),
            );
        }
    }
}

pub(crate) fn loader_settings_meta_transform<S: Settings>(
    settings: impl Fn(&mut S) + Send + Sync + 'static,
) -> MetaTransform {
    Box::new(move |meta| meta_transform_settings(meta, &settings))
}

pub type AssetHash = [u8; 32];

/// NOTE: changing the hashing logic here is a _breaking change_ that requires a [`META_FORMAT_VERSION`] bump.
pub(crate) async fn get_asset_hash(
    meta_bytes: &[u8],
    asset_reader: &mut impl Reader,
) -> Result<AssetHash, AssetReaderError> {
    let mut hasher = blake3::Hasher::new();
    hasher.update(meta_bytes);
    let mut buffer = [0; blake3::CHUNK_LEN];
    loop {
        let bytes_read = asset_reader.read(&mut buffer).await?;
        if bytes_read == 0 {
            // This means we've reached EOF, so we're done consume asset bytes.
            break;
        }
        hasher.update(&buffer[..bytes_read]);
    }
    Ok(*hasher.finalize().as_bytes())
}

/// NOTE: changing the hashing logic here is a _breaking change_ that requires a [`META_FORMAT_VERSION`] bump.
pub(crate) fn get_full_asset_hash(
    asset_hash: AssetHash,
    dependency_hashes: impl Iterator<Item = AssetHash>,
) -> AssetHash {
    let mut hasher = blake3::Hasher::new();
    hasher.update(&asset_hash);
    for hash in dependency_hashes {
        hasher.update(&hash);
    }
    *hasher.finalize().as_bytes()
}


Review comment (suggestion):
```suggestion
            // This means we've reached EOF, so we're done consuming asset bytes.
```

Code after:
                core::any::type_name::<S>(),
            );
        }
    }
}

pub(crate) fn loader_settings_meta_transform<S: Settings>(
    settings: impl Fn(&mut S) + Send + Sync + 'static,
) -> MetaTransform {
    Box::new(move |meta| meta_transform_settings(meta, &settings))
}

pub type AssetHash = [u8; 32];

/// NOTE: changing the hashing logic here is a _breaking change_ that requires a [`META_FORMAT_VERSION`] bump.
pub(crate) async fn get_asset_hash(
    meta_bytes: &[u8],
    asset_reader: &mut impl Reader,
) -> Result<AssetHash, AssetReaderError> {
    let mut hasher = blake3::Hasher::new();
    hasher.update(meta_bytes);
    let mut buffer = [0; blake3::CHUNK_LEN];
    loop {
        let bytes_read = asset_reader.read(&mut buffer).await?;
        hasher.update(&buffer[..bytes_read]);
        if bytes_read < buffer.len() {
            // This means we've reached EOF, so we're done consuming asset bytes.
            break;
        }
    }
    Ok(*hasher.finalize().as_bytes())
}

/// NOTE: changing the hashing logic here is a _breaking change_ that requires a [`META_FORMAT_VERSION`] bump.
pub(crate) fn get_full_asset_hash(
    asset_hash: AssetHash,
    dependency_hashes: impl Iterator<Item = AssetHash>,
) -> AssetHash {
    let mut hasher = blake3::Hasher::new();
    hasher.update(&asset_hash);
    for hash in dependency_hashes {
        hasher.update(&hash);
    }
    *hasher.finalize().as_bytes()
}
Repository: bevyengine/bevy
File: crates/bevy_asset/src/processor/process.rs (Rust)

Code before:
                self.path,
                &meta,
                &*loader,
                &mut self.reader,
                false,
                true,
            )
            .await?;
        for (path, full_hash) in &loaded_asset.loader_dependencies {
            self.new_processed_info
                .process_dependencies
                .push(ProcessDependencyInfo {
                    full_hash: *full_hash,
                    path: path.to_owned(),
                });
        }
        Ok(loaded_asset)
    }

    /// The path of the asset being processed.
    #[inline]
    pub fn path(&self) -> &AssetPath<'static> {
        self.path
    }

    /// The source bytes of the asset being processed.
    #[inline]
    pub fn asset_reader(&mut self) -> &mut dyn Reader {
        &mut self.reader
    }
}


Review comment (suggestion):
```suggestion
    /// The reader for the asset being processed.
```

Code after:
                self.path,
                &meta,
                &*loader,
                &mut self.reader,
                false,
                true,
            )
            .await?;
        for (path, full_hash) in &loaded_asset.loader_dependencies {
            self.new_processed_info
                .process_dependencies
                .push(ProcessDependencyInfo {
                    full_hash: *full_hash,
                    path: path.to_owned(),
                });
        }
        Ok(loaded_asset)
    }

    /// The path of the asset being processed.
    #[inline]
    pub fn path(&self) -> &AssetPath<'static> {
        self.path
    }

    /// The reader for the asset being processed.
    #[inline]
    pub fn asset_reader(&mut self) -> &mut dyn Reader {
        &mut self.reader
    }
}
Repository: bevyengine/bevy
File: crates/bevy_text/src/text.rs (Rust)

Code before:
    }
}

impl From<Handle<Font>> for TextFont {
    fn from(font: Handle<Font>) -> Self {
        Self { font, ..default() }
    }
}

impl Default for TextFont {
    fn default() -> Self {
        Self {
            font: Default::default(),
            font_size: 20.0,
            weight: FontWeight::NORMAL,
            font_features: FontFeatures::default(),
            font_smoothing: Default::default(),
        }
    }
}

/// How thick or bold the strokes of a font appear.
///
/// Valid font weights range from 1 to 1000, inclusive.
/// Weights above 1000 are clamped to 1000.
/// A weight of 0 is treated as `Weight::DEFAULT`.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Reflect)]
pub struct FontWeight(pub u16);

impl FontWeight {
    /// Weight 100.
    pub const THIN: FontWeight = FontWeight(100);

    /// Weight 200.
    pub const EXTRA_LIGHT: FontWeight = FontWeight(200);

    /// Weight 300.
    pub const LIGHT: FontWeight = FontWeight(300);

    /// Weight 400.
    pub const NORMAL: FontWeight = FontWeight(400);

    /// Weight 500.
    pub const MEDIUM: FontWeight = FontWeight(500);

    /// Weight 600.
    pub const SEMIBOLD: FontWeight = FontWeight(600);

    /// Weight 700.
    pub const BOLD: FontWeight = FontWeight(700);


Review comment (suggestion):
We can probably use doc links here: 

```suggestion
/// A weight of 0 is treated as [`FontWeight::DEFAULT`].
```

Or Normal, to avoid the indirection

Code after:
        self.font_smoothing = font_smoothing;
        self
    }
}

impl From<Handle<Font>> for TextFont {
    fn from(font: Handle<Font>) -> Self {
        Self { font, ..default() }
    }
}

impl Default for TextFont {
    fn default() -> Self {
        Self {
            font: Default::default(),
            font_size: 20.0,
            weight: FontWeight::NORMAL,
            font_features: FontFeatures::default(),
            font_smoothing: Default::default(),
        }
    }
}

/// How thick or bold the strokes of a font appear.
///
/// Valid font weights range from 1 to 1000, inclusive.
/// Weights above 1000 are clamped to 1000.
/// A weight of 0 is treated as [`FontWeight::DEFAULT`].
///
/// `<https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/font-weight>`
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Reflect)]
pub struct FontWeight(pub u16);

impl FontWeight {
    /// Weight 100.
    pub const THIN: FontWeight = FontWeight(100);

    /// Weight 200.
    pub const EXTRA_LIGHT: FontWeight = FontWeight(200);

    /// Weight 300.
    pub const LIGHT: FontWeight = FontWeight(300);

    /// Weight 400.
    pub const NORMAL: FontWeight = FontWeight(400);

    /// Weight 500.
    pub const MEDIUM: FontWeight = FontWeight(500);

    /// Weight 600.
    pub const SEMIBOLD: FontWeight = FontWeight(600);
Repository: bevyengine/bevy
File: crates/bevy_text/src/text.rs (Rust)

Code before:
}

impl From<Handle<Font>> for TextFont {
    fn from(font: Handle<Font>) -> Self {
        Self { font, ..default() }
    }
}

impl Default for TextFont {
    fn default() -> Self {
        Self {
            font: Default::default(),
            font_size: 20.0,
            weight: FontWeight::NORMAL,
            font_features: FontFeatures::default(),
            font_smoothing: Default::default(),
        }
    }
}

/// How thick or bold the strokes of a font appear.
///
/// Valid font weights range from 1 to 1000, inclusive.
/// Weights above 1000 are clamped to 1000.
/// A weight of 0 is treated as `Weight::DEFAULT`.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Reflect)]
pub struct FontWeight(pub u16);

impl FontWeight {
    /// Weight 100.
    pub const THIN: FontWeight = FontWeight(100);

    /// Weight 200.
    pub const EXTRA_LIGHT: FontWeight = FontWeight(200);

    /// Weight 300.
    pub const LIGHT: FontWeight = FontWeight(300);

    /// Weight 400.
    pub const NORMAL: FontWeight = FontWeight(400);

    /// Weight 500.
    pub const MEDIUM: FontWeight = FontWeight(500);

    /// Weight 600.
    pub const SEMIBOLD: FontWeight = FontWeight(600);

    /// Weight 700.
    pub const BOLD: FontWeight = FontWeight(700);

    /// Weight 800

Review comment (refactor):
Nit: I appreciate that font weights are well defined, I'm not too familiar with the concept, so it felt a bit like magic numbers, maybe consider linking or hinting to what spec was followed ? I guess it's taken mostly from cosmic text, but if we want to offer an "abstraction", a little more background for that may be welcome: is that css, or is there some other font weight specification ?

Code after:
        self
    }
}

impl From<Handle<Font>> for TextFont {
    fn from(font: Handle<Font>) -> Self {
        Self { font, ..default() }
    }
}

impl Default for TextFont {
    fn default() -> Self {
        Self {
            font: Default::default(),
            font_size: 20.0,
            weight: FontWeight::NORMAL,
            font_features: FontFeatures::default(),
            font_smoothing: Default::default(),
        }
    }
}

/// How thick or bold the strokes of a font appear.
///
/// Valid font weights range from 1 to 1000, inclusive.
/// Weights above 1000 are clamped to 1000.
/// A weight of 0 is treated as [`FontWeight::DEFAULT`].
///
/// `<https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/font-weight>`
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Reflect)]
pub struct FontWeight(pub u16);

impl FontWeight {
    /// Weight 100.
    pub const THIN: FontWeight = FontWeight(100);

    /// Weight 200.
    pub const EXTRA_LIGHT: FontWeight = FontWeight(200);

    /// Weight 300.
    pub const LIGHT: FontWeight = FontWeight(300);

    /// Weight 400.
    pub const NORMAL: FontWeight = FontWeight(400);

    /// Weight 500.
    pub const MEDIUM: FontWeight = FontWeight(500);

    /// Weight 600.
    pub const SEMIBOLD: FontWeight = FontWeight(600);