sburris0 / bevy_flycam Goto Github PK

Working on the Bevy 0.8 release of bevy_atmosphere (link to current commit tree). I noticed that when I run my example (cargo run --example cycle --feature dynamic) and close the window, it actually crashes. I get the following error log:

2022-08-03T00:13:27.505579Z  INFO bevy_winit: Skipped event for closed window: WindowId(00000000-0000-0000-0000-000000000000)
2022-08-03T00:13:27.506219Z  INFO bevy_winit: Skipped event for closed window: WindowId(00000000-0000-0000-0000-000000000000)
2022-08-03T00:13:27.509384Z  INFO bevy_winit: Skipped event for closed window: WindowId(00000000-0000-0000-0000-000000000000)
thread 'Compute Task Pool (1)' panicked at 'called `Option::unwrap()` on a `None` value', C:\Users\jonah\.cargo\registry\src\github.com-1ecc6299db9ec823\bevy_flycam-0.8.0\src\lib.rs:118:44
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at 'task has failed', C:\Users\jonah\.cargo\registry\src\github.com-1ecc6299db9ec823\async-task-4.3.0\src\task.rs:426:45

The error comes from the following code:

fn cursor_grab(keys: Res<Input<KeyCode>>, mut windows: ResMut<Windows>) {
    let window = windows.get_primary_mut().unwrap();
    if keys.just_pressed(KeyCode::Escape) {
        toggle_grab_cursor(window);
    }
}

I believe changing this to something like the following will fix this:

fn cursor_grab(keys: Res<Input<KeyCode>>, mut windows: ResMut<Windows>) {
    if let Some(window) = windows.get_primary_mut() {
        if keys.just_pressed(KeyCode::Escape) {
            toggle_grab_cursor(window);
        }
    }
}

I can fork and create a PR later, just figured it would be good to make an issue first.

Could you change anything with "player" in the title to "freecam" or "fly_cam"? Considering this is for a fly camera it makes sense for it to not be labled as player.

It would be nice, if the initial position and orientation could be set by the user.
May be the MovementSettings could become CameraSettings?
(And thank you for the crate, it gave me a fast help)

Bevy 0.13 recently released, and it would be great to use this crate with it. I'm not fully sure how hard upgrading it would be, but shouldn't be too hard in theory.

If you want, I can take a shot at implementing this, and will create a pr.

Thanks

According to the bevy-cheatbook,

• The X axis goes from left to right (+X points right).
• The Y axis goes from bottom to top (+Y points up).
• The Z axis goes from far to near (+Z points towards you, out of the screen).

I noticed, however, while using the flycam that the A & D keys slide the camera on the local X axis, W & D on the Z axis, and Shift & Space on the Y axis. Because of this, I ended up modeling my world in XZY space, which caused problems later down the line for me. Shouldn't Z be treated as the "up and down" axis, rather than Y, for the sake of consistency?

Hi :)

Very useful camera controller. I would just make a suggestion to make the NoCameraPlayerPlugin not default to grabbing the cursor, but to wait for an Entity with a FlyCam to be added to the world. This would make it easier to navigate menus, click "Play", then have spawn a camera with FlyCam and trigger the cursor grab.

I can make a quick PR if you want :) Thanks!

Since the update to bevy 0.9 and the according flycam version 0.9 the cursor grabbing doesn't work anymore.
Instead the following error will get logged:

ERROR bevy_winit: Unable to un/grab cursor: the requested operation is not supported by Winit

The cursor is invisible while inside the window but as soon as the cursor leaves the window it appears again.
According to bevyengine/bevy#6590 it seems to be that the current mode Confined is only supported by Windows and not macOS, while Locked is only supported by macOS and not Windows.

As the query is only look for changes and chane detection is not trigged on init, the camera doesn't get grabbed.

Something like this (I've already got it to work locally):
Its using 'static lifetime elision, so that we don't need to store anything in the heap.
Changes to lib.rs:

pub struct KeyMap { 
    pub forward: &'static[KeyCode],
    pub backward: &'static[KeyCode],
    pub left: &'static[KeyCode],
    pub right: &'static[KeyCode],
    pub up: &'static[KeyCode],
    pub down: &'static[KeyCode],
}

impl Default for KeyMap {
    fn default() -> Self {
        Self {
            forward: &[KeyCode::W],
            backward: &[KeyCode::S],
            left: &[KeyCode::A],
            right: &[KeyCode::D],
            up: &[KeyCode::Space],
            down: &[KeyCode::LShift],
        }
    }
}

/// Mouse sensitivity and movement speed
pub struct MovementSettings {
    pub sensitivity: f32,
    pub speed: f32,
    pub map: KeyMap,
}

impl Default for MovementSettings {
    fn default() -> Self {
        Self {
            sensitivity: 0.00012,
            speed: 12.,
            map: KeyMap::default()
        }
    }
}

fn validate_key(codes:&'static[KeyCode], key: &KeyCode) -> bool {
    codes.iter().any(|m| m == key)
}

fn player_move(
    keys: Res<Input<KeyCode>>,
    time: Res<Time>,
    windows: Res<Windows>,
    settings: Res<MovementSettings>,
    mut query: Query<(&FlyCam, &mut Transform)>,
) {
    let window = windows.get_primary().unwrap();
    for (_camera, mut transform) in query.iter_mut() {
        let mut velocity = Vec3::ZERO;
        let local_z = transform.local_z();
        let forward = -Vec3::new(local_z.x, 0., local_z.z);
        let right = Vec3::new(local_z.z, 0., -local_z.x);

        for key in keys.get_pressed() {
            if window.cursor_locked() {
				//----- Changes here ------\\
                if validate_key(settings.map.forward,  key) {velocity += forward }
                if validate_key(settings.map.backward, key) {velocity -= forward }
                if validate_key(settings.map.left,     key) {velocity -= right   }
                if validate_key(settings.map.right,    key) {velocity += right   }
                if validate_key(settings.map.up,       key) {velocity += unit_y()}
                if validate_key(settings.map.down,     key) {velocity -= unit_y()}            
            }
        }

        velocity = velocity.normalize();

        if !velocity.is_nan() {
            transform.translation += velocity * time.delta_seconds() * settings.speed
        }
    }
}

Usage (or using default)

.insert_resource(MovementSettings {
    sensitivity: 0.00015, // default: 0.00012
    speed: 12.0, // default: 12.0
    map: KeyMap {
        forward: &[KeyCode::Up, KeyCode::W],
        ..Default::default()
    }
})

The question is: Does it add unnecessary complexity that takes away the simplicity of the library for the benefit of the user? On one hand, it's an opt-in feature and not a breaking feature if you are only using defaults. On the other hand, you still have breaking changes for any have something custom using the lib.

Looking up and down (pitch) is a bit slower than left and right (yaw), which feels weird in my experience. It a lot slower the more wide the window (maybe too slow if fullscreen on a 21:9).

I think this is caused by multiplying the sensitivity and delta by the window.height() or * window.width(). After removing this multiplication, I get a consistent sensitivity in all directions. May I ask why this is being done?

Following queries can be simplified using With filter, since FlyCam is just a marker component.

     mut query: Query<&mut Transform, With<FlyCam>>,
 ) { 
     let window = windows.get_primary().unwrap(); 
     for mut transform in query.iter_mut() {