bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.35k stars 3.49k forks source link

`RegularPolygon` should allow `circumradius` to be 0.0 #13332

Closed mamekoro closed 4 months ago

mamekoro commented 4 months ago

What problem does this solve or what need does it fill?

It currently panics when circumradius is zero.

https://github.com/bevyengine/bevy/blob/443ce9a62b6bb0136c192a6fe46e4c254a895e6c/crates/bevy_math/src/primitives/dim2.rs#L729

Is there any advantage to rejecting 0.0?

Since the value 0.0 is frequently used as a default or minimum value, not allowing it would increase the chances of the game panicking.

Consider, for example, an animation that smoothly resizes a circle. When the circle appears, the radius increases from zero, and when the circle disappears, the radius decreases toward zero. Implementing this animation in v0.12 is simple, but v0.13 requires additional code to make the radius greater than zero to prevent panic.

What solution would you like?

That assertion should be circumradius >= 0.0 to allow zero.

Additional context

Bevy v0.12 can handle a circle of radius 0.0, but v0.13 panics.

use bevy::prelude::*;

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

fn startup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<ColorMaterial>>,
) {
    commands.spawn(Camera2dBundle::default());
    commands.spawn((
        // v0.12 (no panic)
        // ColorMesh2dBundle {
        //     mesh: meshes.add(shape::Circle::new(0.0).into()).into(),
        //     material: materials.add(Color::WHITE.into()),
        //     ..default()
        // },

        // v0.13 (panics)
        ColorMesh2dBundle {
            mesh: meshes.add(Circle::new(0.0)).into(),
            material: materials.add(Color::WHITE),
            ..default()
        },
    ));
}
alice-i-cecile commented 4 months ago

Agreed, I'm fully on board with this change. We should ensure that all of our shapes can scale down to (but not past) 0 in the same PR.