Simplify design for *Label types #4954
Description
Bevy currently has several different kinds of label type, all of which are essentially duplications of each other. For the sake of brevity, I will collectively refer to these as just Label.
What problem does this solve or what need does it fill?
The current design for Label is overly complex and "heavy". The design is centered around trait objects and boxing, which makes for poor ergonomics, and adds overhead to simple comparisons between labels. Since this is such a common operation, we should strive to make it as cheap as possible.
What solution would you like?
To guide the design, let's check out the current docs for Label:
Defines a set of strongly-typed labels for a class of objects
- It's a label, so it needs text.
- It's a set, so many different types must be coercible to it.
- It's strongly typed, so two distinct labels with the same text must not collide.
Label is now a data struct, copy-able. It keeps track of the type it was made with and stores a string reference.
use std::any::TypeId;
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
pub struct Label(TypeId, &'static str);
impl Debug/Display for Label { /* ... */ }
pub trait IntoLabel: Copy + 'static {
fn into_label(self) -> Label {
let id = TypeId::of::<Self>();
let text = self.as_str();
Self(id, text)
}
fn as_str(&self) -> &'static str;
}The 'static requirement for the text may be alarming to some, but I believe the tradeoff is well worth it. It greatly simplifies the implementation and leads to less code being generated and executed, since there's no generic magic, dynamic dispatch, or even branches. And the restriction shouldn't be a problem in most cases, since Labels are usually unit structs, c-style enums, and fn items -- all of these types can easily be represented by a string literal.
#[derive(Clone, Copy)]
pub enum SomeLabels {
Foo,
Bar
}
// result of #[derive(IntoLabel)]
impl IntoLabel for SomeLabel {
fn as_str(&self) -> &'static str {
match self {
Self::Foo => "Foo",
Self::Bar => "Bar",
}
}
}
fn some_system() {
// ...
}
// as part of some giant macro to implement for all Fn types...
impl<F: FnMut() + Copy + 'static> IntoLabel for F {
fn as_str(&self) -> &'static str {
std::any::type_name::<F>()
}
}In cases when a user really needs runtime Labels, they could always just leak a string to get a 'static reference.
What alternative(s) have you considered?
Don't change the design. The old design may not be perfect but it has worked so far.