Skip to content

Use let else #8358

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Use let else #8358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2109911
fix clippy::manual_let_else
B-Reif Apr 11, 2023
a0b07d1
port large blocks to let-else
B-Reif Apr 11, 2023
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
174 changes: 87 additions & 87 deletions crates/bevy_animation/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,102 +434,102 @@ fn apply_animation(
parents: &Query<(Option<With<AnimationPlayer>>, Option<&Parent>)>,
children: &Query<&Children>,
) {
if let Some(animation_clip) = animations.get(&animation.animation_clip) {
if !paused {
animation.elapsed += time.delta_seconds() * animation.speed;
}
let mut elapsed = animation.elapsed;
if animation.repeat {
elapsed %= animation_clip.duration;
}
if elapsed < 0.0 {
elapsed += animation_clip.duration;
}
if animation.path_cache.len() != animation_clip.paths.len() {
animation.path_cache = vec![Vec::new(); animation_clip.paths.len()];
}
if !verify_no_ancestor_player(maybe_parent, parents) {
warn!("Animation player on {:?} has a conflicting animation player on an ancestor. Cannot safely animate.", root);
return;
}

for (path, bone_id) in &animation_clip.paths {
let cached_path = &mut animation.path_cache[*bone_id];
let curves = animation_clip.get_curves(*bone_id).unwrap();
let Some(target) = find_bone(root, path, children, names, cached_path) else { continue };
// SAFETY: The verify_no_ancestor_player check above ensures that two animation players cannot alias
// any of their descendant Transforms.
//
// The system scheduler prevents any other system from mutating Transforms at the same time,
// so the only way this fetch can alias is if two AnimationPlayers are targeting the same bone.
// This can only happen if there are two or more AnimationPlayers are ancestors to the same
// entities. By verifying that there is no other AnimationPlayer in the ancestors of a
// running AnimationPlayer before animating any entity, this fetch cannot alias.
//
// This means only the AnimationPlayers closest to the root of the hierarchy will be able
// to run their animation. Any players in the children or descendants will log a warning
// and do nothing.
let Ok(mut transform) = (unsafe { transforms.get_unchecked(target) }) else { continue };
for curve in curves {
// Some curves have only one keyframe used to set a transform
if curve.keyframe_timestamps.len() == 1 {
match &curve.keyframes {
Keyframes::Rotation(keyframes) => {
transform.rotation = transform.rotation.slerp(keyframes[0], weight);
}
Keyframes::Translation(keyframes) => {
transform.translation =
transform.translation.lerp(keyframes[0], weight);
}
Keyframes::Scale(keyframes) => {
transform.scale = transform.scale.lerp(keyframes[0], weight);
}
}
continue;
}
let Some(animation_clip) = animations.get(&animation.animation_clip) else {
return;
};
if !paused {
animation.elapsed += time.delta_seconds() * animation.speed;
}
let mut elapsed = animation.elapsed;
if animation.repeat {
elapsed %= animation_clip.duration;
}
if elapsed < 0.0 {
elapsed += animation_clip.duration;
}
if animation.path_cache.len() != animation_clip.paths.len() {
animation.path_cache = vec![Vec::new(); animation_clip.paths.len()];
}
if !verify_no_ancestor_player(maybe_parent, parents) {
warn!("Animation player on {:?} has a conflicting animation player on an ancestor. Cannot safely animate.", root);
return;
}

// Find the current keyframe
// PERF: finding the current keyframe can be optimised
let step_start = match curve
.keyframe_timestamps
.binary_search_by(|probe| probe.partial_cmp(&elapsed).unwrap())
{
Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
Ok(i) => i,
Err(0) => continue, // this curve isn't started yet
Err(n) if n > curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
Err(i) => i - 1,
};
let ts_start = curve.keyframe_timestamps[step_start];
let ts_end = curve.keyframe_timestamps[step_start + 1];
let lerp = (elapsed - ts_start) / (ts_end - ts_start);

// Apply the keyframe
for (path, bone_id) in &animation_clip.paths {
let cached_path = &mut animation.path_cache[*bone_id];
let curves = animation_clip.get_curves(*bone_id).unwrap();
let Some(target) = find_bone(root, path, children, names, cached_path) else { continue };
// SAFETY: The verify_no_ancestor_player check above ensures that two animation players cannot alias
// any of their descendant Transforms.
//
// The system scheduler prevents any other system from mutating Transforms at the same time,
// so the only way this fetch can alias is if two AnimationPlayers are targeting the same bone.
// This can only happen if there are two or more AnimationPlayers are ancestors to the same
// entities. By verifying that there is no other AnimationPlayer in the ancestors of a
// running AnimationPlayer before animating any entity, this fetch cannot alias.
//
// This means only the AnimationPlayers closest to the root of the hierarchy will be able
// to run their animation. Any players in the children or descendants will log a warning
// and do nothing.
let Ok(mut transform) = (unsafe { transforms.get_unchecked(target) }) else { continue };
for curve in curves {
// Some curves have only one keyframe used to set a transform
if curve.keyframe_timestamps.len() == 1 {
match &curve.keyframes {
Keyframes::Rotation(keyframes) => {
let rot_start = keyframes[step_start];
let mut rot_end = keyframes[step_start + 1];
// Choose the smallest angle for the rotation
if rot_end.dot(rot_start) < 0.0 {
rot_end = -rot_end;
}
// Rotations are using a spherical linear interpolation
let rot = rot_start.normalize().slerp(rot_end.normalize(), lerp);
transform.rotation = transform.rotation.slerp(rot, weight);
transform.rotation = transform.rotation.slerp(keyframes[0], weight);
}
Keyframes::Translation(keyframes) => {
let translation_start = keyframes[step_start];
let translation_end = keyframes[step_start + 1];
let result = translation_start.lerp(translation_end, lerp);
transform.translation = transform.translation.lerp(result, weight);
transform.translation = transform.translation.lerp(keyframes[0], weight);
}
Keyframes::Scale(keyframes) => {
let scale_start = keyframes[step_start];
let scale_end = keyframes[step_start + 1];
let result = scale_start.lerp(scale_end, lerp);
transform.scale = transform.scale.lerp(result, weight);
transform.scale = transform.scale.lerp(keyframes[0], weight);
}
}
continue;
}

// Find the current keyframe
// PERF: finding the current keyframe can be optimised
let step_start = match curve
.keyframe_timestamps
.binary_search_by(|probe| probe.partial_cmp(&elapsed).unwrap())
{
Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
Ok(i) => i,
Err(0) => continue, // this curve isn't started yet
Err(n) if n > curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
Err(i) => i - 1,
};
let ts_start = curve.keyframe_timestamps[step_start];
let ts_end = curve.keyframe_timestamps[step_start + 1];
let lerp = (elapsed - ts_start) / (ts_end - ts_start);

// Apply the keyframe
match &curve.keyframes {
Keyframes::Rotation(keyframes) => {
let rot_start = keyframes[step_start];
let mut rot_end = keyframes[step_start + 1];
// Choose the smallest angle for the rotation
if rot_end.dot(rot_start) < 0.0 {
rot_end = -rot_end;
}
// Rotations are using a spherical linear interpolation
let rot = rot_start.normalize().slerp(rot_end.normalize(), lerp);
transform.rotation = transform.rotation.slerp(rot, weight);
}
Keyframes::Translation(keyframes) => {
let translation_start = keyframes[step_start];
let translation_end = keyframes[step_start + 1];
let result = translation_start.lerp(translation_end, lerp);
transform.translation = transform.translation.lerp(result, weight);
}
Keyframes::Scale(keyframes) => {
let scale_start = keyframes[step_start];
let scale_end = keyframes[step_start + 1];
let result = scale_start.lerp(scale_end, lerp);
transform.scale = transform.scale.lerp(result, weight);
}
}
}
}
Expand Down
60 changes: 31 additions & 29 deletions crates/bevy_app/src/plugin_group.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,28 +66,29 @@ impl PluginGroupBuilder {
// Insert the new plugin as enabled, and removes its previous ordering if it was
// already present
fn upsert_plugin_state<T: Plugin>(&mut self, plugin: T, added_at_index: usize) {
if let Some(entry) = self.plugins.insert(
let Some(entry) = self.plugins.insert(
TypeId::of::<T>(),
PluginEntry {
plugin: Box::new(plugin),
enabled: true,
},
) {
if entry.enabled {
warn!(
"You are replacing plugin '{}' that was not disabled.",
entry.plugin.name()
);
}
if let Some(to_remove) = self
.order
.iter()
.enumerate()
.find(|(i, ty)| *i != added_at_index && **ty == TypeId::of::<T>())
.map(|(i, _)| i)
{
self.order.remove(to_remove);
}
) else {
return;
};
if entry.enabled {
warn!(
"You are replacing plugin '{}' that was not disabled.",
entry.plugin.name()
);
}
if let Some(to_remove) = self
.order
.iter()
.enumerate()
.find(|(i, ty)| *i != added_at_index && **ty == TypeId::of::<T>())
.map(|(i, _)| i)
{
self.order.remove(to_remove);
}
}

Expand Down Expand Up @@ -174,18 +175,19 @@ impl PluginGroupBuilder {
/// Panics if one of the plugin in the group was already added to the application.
pub fn finish(mut self, app: &mut App) {
for ty in &self.order {
if let Some(entry) = self.plugins.remove(ty) {
if entry.enabled {
debug!("added plugin: {}", entry.plugin.name());
if let Err(AppError::DuplicatePlugin { plugin_name }) =
app.add_boxed_plugin(entry.plugin)
{
panic!(
"Error adding plugin {} in group {}: plugin was already added in application",
plugin_name,
self.group_name
);
}
let Some(entry) = self.plugins.remove(ty) else {
continue;
};
if entry.enabled {
debug!("added plugin: {}", entry.plugin.name());
if let Err(AppError::DuplicatePlugin { plugin_name }) =
app.add_boxed_plugin(entry.plugin)
{
panic!(
"Error adding plugin {} in group {}: plugin was already added in application",
plugin_name,
self.group_name
);
}
}
}
Expand Down
25 changes: 13 additions & 12 deletions crates/bevy_asset/src/asset_server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,18 +512,19 @@ impl AssetServer {
let asset_sources = self.server.asset_sources.read();
let asset_lifecycles = self.server.asset_lifecycles.read();
for potential_free in potential_frees.drain(..) {
if let Some(&0) = ref_counts.get(&potential_free) {
let type_uuid = match potential_free {
HandleId::Id(type_uuid, _) => Some(type_uuid),
HandleId::AssetPathId(id) => asset_sources
.get(&id.source_path_id())
.and_then(|source_info| source_info.get_asset_type(id.label_id())),
};

if let Some(type_uuid) = type_uuid {
if let Some(asset_lifecycle) = asset_lifecycles.get(&type_uuid) {
asset_lifecycle.free_asset(potential_free);
}
let Some(&0) = ref_counts.get(&potential_free) else {
continue;
};
let type_uuid = match potential_free {
HandleId::Id(type_uuid, _) => Some(type_uuid),
HandleId::AssetPathId(id) => asset_sources
.get(&id.source_path_id())
.and_then(|source_info| source_info.get_asset_type(id.label_id())),
};

if let Some(type_uuid) = type_uuid {
if let Some(asset_lifecycle) = asset_lifecycles.get(&type_uuid) {
asset_lifecycle.free_asset(potential_free);
}
}
}
Expand Down
50 changes: 23 additions & 27 deletions crates/bevy_asset/src/io/file_asset_io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,34 +175,30 @@ impl AssetIo for FileAssetIo {
all(not(target_arch = "wasm32"), not(target_os = "android"))
))]
pub fn filesystem_watcher_system(asset_server: Res<AssetServer>) {
let asset_io =
if let Some(asset_io) = asset_server.server.asset_io.downcast_ref::<FileAssetIo>() {
asset_io
} else {
return;
};
let Some(asset_io) = asset_server.server.asset_io.downcast_ref::<FileAssetIo>() else {
return;
};
let watcher = asset_io.filesystem_watcher.read();
if let Some(ref watcher) = *watcher {
let mut changed = HashSet::<&PathBuf>::default();
loop {
let event = match watcher.receiver.try_recv() {
Ok(result) => result.unwrap(),
Err(TryRecvError::Empty) => break,
Err(TryRecvError::Disconnected) => panic!("FilesystemWatcher disconnected."),
};
if let notify::event::Event {
kind: notify::event::EventKind::Modify(_),
paths,
..
} = event
{
for path in &paths {
let Some(set) = watcher.path_map.get(path) else {continue};
for to_reload in set {
if !changed.contains(to_reload) {
changed.insert(to_reload);
let _ = asset_server.load_untracked(to_reload.as_path().into(), true);
}
let Some(ref watcher) = *watcher else { return; };
let mut changed = HashSet::<&PathBuf>::default();
loop {
let event = match watcher.receiver.try_recv() {
Ok(result) => result.unwrap(),
Err(TryRecvError::Empty) => break,
Err(TryRecvError::Disconnected) => panic!("FilesystemWatcher disconnected."),
};
if let notify::event::Event {
kind: notify::event::EventKind::Modify(_),
paths,
..
} = event
{
for path in &paths {
let Some(set) = watcher.path_map.get(path) else {continue};
for to_reload in set {
if !changed.contains(to_reload) {
changed.insert(to_reload);
let _ = asset_server.load_untracked(to_reload.as_path().into(), true);
}
}
}
Expand Down
Loading