Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panics in scene_viewer and audio_control #16983

Merged
merged 6 commits into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions examples/audio/audio_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,48 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
#[derive(Component)]
struct MyMusic;

fn update_speed(sink: Single<&AudioSink, With<MyMusic>>, time: Res<Time>) {
fn update_speed(music_controller: Query<&AudioSink, With<MyMusic>>, time: Res<Time>) {
let Ok(sink) = music_controller.get_single() else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this? I thought Single here was very intentional and intended to replace this pattern. This seems to just ignore invalid state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single now panics again. Using Option<Single> here would work, but that's kind of a sidegrade.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can reconfigure the system to ignore failing params

Copy link
Contributor Author

@rparrett rparrett Dec 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's expected that the sink won't be present until PostUpdate on the first frame.

Specifying Single and disabling warnings feels like a dirty way to write this code to me. Option<Single> seems even wackier IMO. But please discuss and feel free to close for an alternative implementation.

Or I can fix it up late tonight or tomorrow if there's consensus the other way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution is totally fine, just listing alternatives

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this solution is fine.

return;
};

sink.set_speed((ops::sin(time.elapsed_secs() / 5.0) + 1.0).max(0.1));
}

fn pause(keyboard_input: Res<ButtonInput<KeyCode>>, sink: Single<&AudioSink, With<MyMusic>>) {
fn pause(
keyboard_input: Res<ButtonInput<KeyCode>>,
music_controller: Query<&AudioSink, With<MyMusic>>,
) {
let Ok(sink) = music_controller.get_single() else {
return;
};

if keyboard_input.just_pressed(KeyCode::Space) {
sink.toggle_playback();
}
}

fn mute(
keyboard_input: Res<ButtonInput<KeyCode>>,
mut sink: Single<&mut AudioSink, With<MyMusic>>,
mut music_controller: Query<&mut AudioSink, With<MyMusic>>,
) {
let Ok(mut sink) = music_controller.get_single_mut() else {
return;
};

if keyboard_input.just_pressed(KeyCode::KeyM) {
sink.toggle_mute();
}
}

fn volume(
keyboard_input: Res<ButtonInput<KeyCode>>,
mut sink: Single<&mut AudioSink, With<MyMusic>>,
mut music_controller: Query<&mut AudioSink, With<MyMusic>>,
) {
let Ok(mut sink) = music_controller.get_single_mut() else {
return;
};

if keyboard_input.just_pressed(KeyCode::Equal) {
let current_volume = sink.volume();
sink.set_volume(current_volume + 0.1);
Expand Down
6 changes: 4 additions & 2 deletions examples/helpers/camera_controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,13 @@ fn run_camera_controller(
key_input: Res<ButtonInput<KeyCode>>,
mut toggle_cursor_grab: Local<bool>,
mut mouse_cursor_grab: Local<bool>,
query: Single<(&mut Transform, &mut CameraController), With<Camera>>,
mut query: Query<(&mut Transform, &mut CameraController), With<Camera>>,
) {
let dt = time.delta_secs();

let (mut transform, mut controller) = query.into_inner();
let Ok((mut transform, mut controller)) = query.get_single_mut() else {
return;
};

if !controller.initialized {
let (yaw, pitch, _roll) = transform.rotation.to_euler(EulerRot::YXZ);
Expand Down
31 changes: 20 additions & 11 deletions examples/tools/scene_viewer/morph_viewer_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use crate::scene_viewer_plugin::SceneHandle;
use bevy::prelude::*;
use std::fmt;

const FONT_SIZE: f32 = 13.0;

const WEIGHT_PER_SECOND: f32 = 0.8;
const ALL_MODIFIERS: &[KeyCode] = &[KeyCode::ShiftLeft, KeyCode::ControlLeft, KeyCode::AltLeft];
const AVAILABLE_KEYS: [MorphKey; 56] = [
Expand Down Expand Up @@ -122,8 +124,8 @@ impl fmt::Display for Target {
}
}
impl Target {
fn text_span(&self, key: &str, style: TextFont) -> (String, TextFont) {
(format!("[{key}] {self}\n"), style)
fn text_span(&self, key: &str, style: TextFont) -> (TextSpan, TextFont) {
(TextSpan::new(format!("[{key}] {self}\n")), style)
}
fn new(
entity_name: Option<&Name>,
Expand Down Expand Up @@ -178,13 +180,18 @@ impl MorphKey {
}
fn update_text(
controls: Option<ResMut<WeightsControl>>,
text: Single<Entity, With<Text>>,
texts: Query<Entity, With<Text>>,
morphs: Query<&MorphWeights>,
mut writer: TextUiWriter,
) {
let Some(mut controls) = controls else {
return;
};

let Ok(text) = texts.get_single() else {
return;
};

for (i, target) in controls.weights.iter_mut().enumerate() {
let Ok(weights) = morphs.get(target.entity) else {
continue;
Expand All @@ -196,7 +203,8 @@ fn update_text(
target.weight = actual_weight;
}
let key_name = &AVAILABLE_KEYS[i].name;
*writer.text(*text, i + 3) = format!("[{key_name}] {target}\n");

*writer.text(text, i + 3) = format!("[{key_name}] {target}\n");
}
}
fn update_morphs(
Expand Down Expand Up @@ -254,12 +262,12 @@ fn detect_morphs(
}
detected.truncate(AVAILABLE_KEYS.len());
let style = TextFont {
font_size: 13.0,
font_size: FONT_SIZE,
..default()
};
let mut spans = vec![
("Morph Target Controls\n".into(), style.clone()),
("---------------\n".into(), style.clone()),
(TextSpan::new("Morph Target Controls\n"), style.clone()),
(TextSpan::new("---------------\n"), style.clone()),
];
let target_to_text =
|(i, target): (usize, &Target)| target.text_span(AVAILABLE_KEYS[i].name, style.clone());
Expand All @@ -270,14 +278,15 @@ fn detect_morphs(
Text::default(),
Node {
position_type: PositionType::Absolute,
top: Val::Px(10.0),
left: Val::Px(10.0),
top: Val::Px(12.0),
left: Val::Px(12.0),
..default()
},
))
.with_children(|p| {
p.spawn((TextSpan::new("Morph Target Controls\n"), style.clone()));
p.spawn((TextSpan::new("---------------\n"), style));
for span in spans {
p.spawn(span);
}
});
}

Expand Down
Loading