Add fallible add methods to PluginGroupBuilder (#17005)

# Objective

Make working with `PluginGroupBuilder` less panicky.
Fixes #17001

## Solution

Expand the `PluginGroupBuilder` api with fallible add methods + a
contains method.
Also reorder the `PluginGroupBuilder` tests because before should come
before after.

## Testing

Ran the `PluginGroupBuilder` tests which call into all the newly added
methods.
This commit is contained in:
Brezak 2024-12-30 21:14:02 +01:00 committed by GitHub
parent 7767a8d161
commit ae16bdf172
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -4,7 +4,7 @@ use alloc::{
string::{String, ToString},
vec::Vec,
};
use bevy_utils::TypeIdMap;
use bevy_utils::{hashbrown::hash_map::Entry, TypeIdMap};
use core::any::TypeId;
use log::{debug, warn};
@ -224,11 +224,6 @@ impl PluginGroup for PluginGroupBuilder {
}
}
/// Helper method to get the [`TypeId`] of a value without having to name its type.
fn type_id_of_val<T: 'static>(_: &T) -> TypeId {
TypeId::of::<T>()
}
/// Facilitates the creation and configuration of a [`PluginGroup`].
///
/// Provides a build ordering to ensure that [`Plugin`]s which produce/require a [`Resource`](bevy_ecs::system::Resource)
@ -250,20 +245,24 @@ impl PluginGroupBuilder {
}
}
/// Finds the index of a target [`Plugin`]. Panics if the target's [`TypeId`] is not found.
fn index_of<Target: Plugin>(&self) -> usize {
let index = self
.order
.iter()
.position(|&ty| ty == TypeId::of::<Target>());
/// Checks if the [`PluginGroupBuilder`] contains the given [`Plugin`].
pub fn contains<T: Plugin>(&self) -> bool {
self.plugins.contains_key(&TypeId::of::<T>())
}
match index {
Some(i) => i,
None => panic!(
"Plugin does not exist in group: {}.",
core::any::type_name::<Target>()
),
}
/// Returns `true` if the [`PluginGroupBuilder`] contains the given [`Plugin`] and it's enabled.
pub fn enabled<T: Plugin>(&self) -> bool {
self.plugins
.get(&TypeId::of::<T>())
.map(|e| e.enabled)
.unwrap_or(false)
}
/// Finds the index of a target [`Plugin`].
fn index_of<Target: Plugin>(&self) -> Option<usize> {
self.order
.iter()
.position(|&ty| ty == TypeId::of::<Target>())
}
// Insert the new plugin as enabled, and removes its previous ordering if it was
@ -311,15 +310,27 @@ impl PluginGroupBuilder {
/// # Panics
///
/// Panics if the [`Plugin`] does not exist.
pub fn set<T: Plugin>(mut self, plugin: T) -> Self {
let entry = self.plugins.get_mut(&TypeId::of::<T>()).unwrap_or_else(|| {
pub fn set<T: Plugin>(self, plugin: T) -> Self {
self.try_set(plugin).unwrap_or_else(|_| {
panic!(
"{} does not exist in this PluginGroup",
core::any::type_name::<T>(),
)
});
entry.plugin = Box::new(plugin);
self
})
}
/// Tries to set the value of the given [`Plugin`], if it exists.
///
/// If the given plugin doesn't exist returns self and the passed in [`Plugin`].
pub fn try_set<T: Plugin>(mut self, plugin: T) -> Result<Self, (Self, T)> {
match self.plugins.entry(TypeId::of::<T>()) {
Entry::Occupied(mut entry) => {
entry.get_mut().plugin = Box::new(plugin);
Ok(self)
}
Entry::Vacant(_) => Err((self, plugin)),
}
}
/// Adds the plugin [`Plugin`] at the end of this [`PluginGroupBuilder`]. If the plugin was
@ -336,6 +347,17 @@ impl PluginGroupBuilder {
self
}
/// Attempts to add the plugin [`Plugin`] at the end of this [`PluginGroupBuilder`].
///
/// If the plugin was already in the group the addition fails.
pub fn try_add<T: Plugin>(self, plugin: T) -> Result<Self, (Self, T)> {
if self.contains::<T>() {
return Err((self, plugin));
}
Ok(self.add(plugin))
}
/// Adds a [`PluginGroup`] at the end of this [`PluginGroupBuilder`]. If the plugin was
/// already in the group, it is removed from its previous place.
pub fn add_group(mut self, group: impl PluginGroup) -> Self {
@ -357,23 +379,105 @@ impl PluginGroupBuilder {
}
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] before the plugin of type `Target`.
/// If the plugin was already the group, it is removed from its previous place. There must
/// be a plugin of type `Target` in the group or it will panic.
pub fn add_before<Target: Plugin>(mut self, plugin: impl Plugin) -> Self {
let target_index = self.index_of::<Target>();
self.order.insert(target_index, type_id_of_val(&plugin));
///
/// If the plugin was already the group, it is removed from its previous place.
///
/// # Panics
///
/// Panics if `Target` is not already in this [`PluginGroupBuilder`].
pub fn add_before<Target: Plugin>(self, plugin: impl Plugin) -> Self {
self.try_add_before_overwrite::<Target, _>(plugin)
.unwrap_or_else(|_| {
panic!(
"Plugin does not exist in group: {}.",
core::any::type_name::<Target>()
)
})
}
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] before the plugin of type `Target`.
///
/// If the plugin was already in the group the add fails. If there isn't a plugin
/// of type `Target` in the group the plugin we're trying to insert is returned.
pub fn try_add_before<Target: Plugin, Insert: Plugin>(
self,
plugin: Insert,
) -> Result<Self, (Self, Insert)> {
if self.contains::<Insert>() {
return Err((self, plugin));
}
self.try_add_before_overwrite::<Target, _>(plugin)
}
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] before the plugin of type `Target`.
///
/// If the plugin was already in the group, it is removed from its previous places.
/// If there isn't a plugin of type `Target` in the group the plugin we're trying to insert
/// is returned.
pub fn try_add_before_overwrite<Target: Plugin, Insert: Plugin>(
mut self,
plugin: Insert,
) -> Result<Self, (Self, Insert)> {
let Some(target_index) = self.index_of::<Target>() else {
return Err((self, plugin));
};
self.order.insert(target_index, TypeId::of::<Insert>());
self.upsert_plugin_state(plugin, target_index);
self
Ok(self)
}
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] after the plugin of type `Target`.
/// If the plugin was already the group, it is removed from its previous place. There must
/// be a plugin of type `Target` in the group or it will panic.
pub fn add_after<Target: Plugin>(mut self, plugin: impl Plugin) -> Self {
let target_index = self.index_of::<Target>() + 1;
self.order.insert(target_index, type_id_of_val(&plugin));
///
/// If the plugin was already the group, it is removed from its previous place.
///
/// # Panics
///
/// Panics if `Target` is not already in this [`PluginGroupBuilder`].
pub fn add_after<Target: Plugin>(self, plugin: impl Plugin) -> Self {
self.try_add_after_overwrite::<Target, _>(plugin)
.unwrap_or_else(|_| {
panic!(
"Plugin does not exist in group: {}.",
core::any::type_name::<Target>()
)
})
}
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] after the plugin of type `Target`.
///
/// If the plugin was already in the group the add fails. If there isn't a plugin
/// of type `Target` in the group the plugin we're trying to insert is returned.
pub fn try_add_after<Target: Plugin, Insert: Plugin>(
self,
plugin: Insert,
) -> Result<Self, (Self, Insert)> {
if self.contains::<Insert>() {
return Err((self, plugin));
}
self.try_add_after_overwrite::<Target, _>(plugin)
}
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] after the plugin of type `Target`.
///
/// If the plugin was already in the group, it is removed from its previous places.
/// If there isn't a plugin of type `Target` in the group the plugin we're trying to insert
/// is returned.
pub fn try_add_after_overwrite<Target: Plugin, Insert: Plugin>(
mut self,
plugin: Insert,
) -> Result<Self, (Self, Insert)> {
let Some(target_index) = self.index_of::<Target>() else {
return Err((self, plugin));
};
let target_index = target_index + 1;
self.order.insert(target_index, TypeId::of::<Insert>());
self.upsert_plugin_state(plugin, target_index);
self
Ok(self)
}
/// Enables a [`Plugin`].
@ -451,6 +555,8 @@ impl PluginGroup for NoopPluginGroup {
#[cfg(test)]
mod tests {
use core::{any::TypeId, fmt::Debug};
use super::PluginGroupBuilder;
use crate::{App, NoopPluginGroup, Plugin};
@ -469,6 +575,35 @@ mod tests {
fn build(&self, _: &mut App) {}
}
#[derive(PartialEq, Debug)]
struct PluginWithData(u32);
impl Plugin for PluginWithData {
fn build(&self, _: &mut App) {}
}
fn get_plugin<T: Debug + 'static>(group: &PluginGroupBuilder, id: TypeId) -> &T {
group.plugins[&id]
.plugin
.as_any()
.downcast_ref::<T>()
.unwrap()
}
#[test]
fn contains() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add(PluginB);
assert!(group.contains::<PluginA>());
assert!(!group.contains::<PluginC>());
let group = group.disable::<PluginA>();
assert!(group.enabled::<PluginB>());
assert!(!group.enabled::<PluginA>());
}
#[test]
fn basic_ordering() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
@ -479,26 +614,9 @@ mod tests {
assert_eq!(
group.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginB>(),
core::any::TypeId::of::<PluginC>(),
]
);
}
#[test]
fn add_after() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add(PluginB)
.add_after::<PluginA>(PluginC);
assert_eq!(
group.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginC>(),
core::any::TypeId::of::<PluginB>(),
TypeId::of::<PluginA>(),
TypeId::of::<PluginB>(),
TypeId::of::<PluginC>(),
]
);
}
@ -513,9 +631,146 @@ mod tests {
assert_eq!(
group.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginC>(),
core::any::TypeId::of::<PluginB>(),
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
TypeId::of::<PluginB>(),
]
);
}
#[test]
fn try_add_before() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>().add(PluginA);
let Ok(group) = group.try_add_before::<PluginA, _>(PluginC) else {
panic!("PluginA wasn't in group");
};
assert_eq!(
group.order,
vec![TypeId::of::<PluginC>(), TypeId::of::<PluginA>(),]
);
assert!(group.try_add_before::<PluginA, _>(PluginC).is_err());
}
#[test]
#[should_panic(
expected = "Plugin does not exist in group: bevy_app::plugin_group::tests::PluginB."
)]
fn add_before_nonexistent() {
PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add_before::<PluginB>(PluginC);
}
#[test]
fn add_after() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add(PluginB)
.add_after::<PluginA>(PluginC);
assert_eq!(
group.order,
vec![
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
TypeId::of::<PluginB>(),
]
);
}
#[test]
fn try_add_after() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add(PluginB);
let Ok(group) = group.try_add_after::<PluginA, _>(PluginC) else {
panic!("PluginA wasn't in group");
};
assert_eq!(
group.order,
vec![
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
TypeId::of::<PluginB>(),
]
);
assert!(group.try_add_after::<PluginA, _>(PluginC).is_err());
}
#[test]
#[should_panic(
expected = "Plugin does not exist in group: bevy_app::plugin_group::tests::PluginB."
)]
fn add_after_nonexistent() {
PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add_after::<PluginB>(PluginC);
}
#[test]
fn add_overwrite() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add(PluginWithData(0x0F))
.add(PluginC);
let id = TypeId::of::<PluginWithData>();
assert_eq!(
get_plugin::<PluginWithData>(&group, id),
&PluginWithData(0x0F)
);
let group = group.add(PluginWithData(0xA0));
assert_eq!(
get_plugin::<PluginWithData>(&group, id),
&PluginWithData(0xA0)
);
assert_eq!(
group.order,
vec![
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
TypeId::of::<PluginWithData>(),
]
);
let Ok(group) = group.try_add_before_overwrite::<PluginA, _>(PluginWithData(0x01)) else {
panic!("PluginA wasn't in group");
};
assert_eq!(
get_plugin::<PluginWithData>(&group, id),
&PluginWithData(0x01)
);
assert_eq!(
group.order,
vec![
TypeId::of::<PluginWithData>(),
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
]
);
let Ok(group) = group.try_add_after_overwrite::<PluginA, _>(PluginWithData(0xdeadbeef))
else {
panic!("PluginA wasn't in group");
};
assert_eq!(
get_plugin::<PluginWithData>(&group, id),
&PluginWithData(0xdeadbeef)
);
assert_eq!(
group.order,
vec![
TypeId::of::<PluginA>(),
TypeId::of::<PluginWithData>(),
TypeId::of::<PluginC>(),
]
);
}
@ -531,27 +786,9 @@ mod tests {
assert_eq!(
group.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginC>(),
core::any::TypeId::of::<PluginB>(),
]
);
}
#[test]
fn readd_after() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add(PluginB)
.add(PluginC)
.add_after::<PluginA>(PluginC);
assert_eq!(
group.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginC>(),
core::any::TypeId::of::<PluginB>(),
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
TypeId::of::<PluginB>(),
]
);
}
@ -567,9 +804,27 @@ mod tests {
assert_eq!(
group.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginC>(),
core::any::TypeId::of::<PluginB>(),
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
TypeId::of::<PluginB>(),
]
);
}
#[test]
fn readd_after() {
let group = PluginGroupBuilder::start::<NoopPluginGroup>()
.add(PluginA)
.add(PluginB)
.add(PluginC)
.add_after::<PluginA>(PluginC);
assert_eq!(
group.order,
vec![
TypeId::of::<PluginA>(),
TypeId::of::<PluginC>(),
TypeId::of::<PluginB>(),
]
);
}
@ -587,9 +842,9 @@ mod tests {
assert_eq!(
group_b.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginB>(),
core::any::TypeId::of::<PluginC>(),
TypeId::of::<PluginA>(),
TypeId::of::<PluginB>(),
TypeId::of::<PluginC>(),
]
);
}
@ -611,9 +866,9 @@ mod tests {
assert_eq!(
group.order,
vec![
core::any::TypeId::of::<PluginA>(),
core::any::TypeId::of::<PluginB>(),
core::any::TypeId::of::<PluginC>(),
TypeId::of::<PluginA>(),
TypeId::of::<PluginB>(),
TypeId::of::<PluginC>(),
]
);
}