Panic messages for systems are way too long sometimes

[JoJoJet]

Bevy version

0.11

What you did

I ran a system that accesses a resource which doesn't exist.

What went wrong

I got this message:

thread 'Compute Task Pool (11)' panicked at 'Resource requested by creator::library::show_library<fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::system::query::Query<(bevy_ecs::entity::Entity, &model_library::LibEntry, core::option::Option<&model_library::Thumbnail>, core::option::Option<&model_library::ProcessedData>, bevy_ecs::query::fetch::Has<model_library::PoserTask>)>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>), fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::system::query::Query<(bevy_ecs::entity::Entity, &model_library::LibEntry, core::option::Option<&model_library::Thumbnail>, core::option::Option<&model_library::Checkpoint>, bevy_ecs::query::fetch::Has<model_library::TrainingTask>, core::option::Option<&model_library::TrainingPort>)>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>, bevy_ecs::event::EventWriter<model_library::StartTraining>, bevy_ecs::event::EventWriter<comm::TryConnect>), fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::system::query::Query<&model_library::TrainingPort, bevy_ecs::query::filter::Added<model_library::TrainingPort>>, bevy_ecs::event::EventWriter<comm::TryConnect>), fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::Res<bevy_input::input::Input<bevy_input::keyboard::KeyCode>>, bevy_ecs::change_detection::ResMut<creator::library::DroppedFile>, bevy_ecs::system::system_param::Local<creator::library::PromptState>, bevy_ecs::event::EventWriter<model_library::StartPoser>, bevy_ecs::event::EventWriter<creator::upload::StartUpload>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>), fn(bevy_ecs::system::query::Query<(&model_library::LibEntry, &model_library::Checkpoint, core::option::Option<&model_library::Thumbnail>)>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::ResMut<comm::Client>), creator::library::browse, creator::library::select, creator::library::wait, creator::library::upload, creator::library::import_model>::{{closure}} does not exist: comm::Client'

The message should have looked something like:

thread 'Compute Task Pool (11)' panicked at 'Resource requested by creator::library::show_library does not exist: comm::Client'

Additional information

The system in question uses SystemParamFunction in order to create composite systems. Its definition looks something like

fn show_library<System1, System2, M1, M2>(
  system1: System1,
  system2: System2
) -> impl FnMut(ParamSet<(System1::Param, System2::Param)>)
where
  System1: SystemParamFunction<M1>,
  System2: SystemParamFunction<M2>,

It is added to the schedule like

.add_systems(show_library(system_1, system_2))

[bushrat011899]

For those working on a solution, the current type-name for that system is:

creator::library::show_library<fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::system::query::Query<(bevy_ecs::entity::Entity, &model_library::LibEntry, core::option::Option<&model_library::Thumbnail>, core::option::Option<&model_library::ProcessedData>, bevy_ecs::query::fetch::Has<model_library::PoserTask>)>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>), fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::system::query::Query<(bevy_ecs::entity::Entity, &model_library::LibEntry, core::option::Option<&model_library::Thumbnail>, core::option::Option<&model_library::Checkpoint>, bevy_ecs::query::fetch::Has<model_library::TrainingTask>, core::option::Option<&model_library::TrainingPort>)>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>, bevy_ecs::event::EventWriter<model_library::StartTraining>, bevy_ecs::event::EventWriter<comm::TryConnect>), fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::system::query::Query<&model_library::TrainingPort, bevy_ecs::query::filter::Added<model_library::TrainingPort>>, bevy_ecs::event::EventWriter<comm::TryConnect>), fn(bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::Res<bevy_input::input::Input<bevy_input::keyboard::KeyCode>>, bevy_ecs::change_detection::ResMut<creator::library::DroppedFile>, bevy_ecs::system::system_param::Local<creator::library::PromptState>, bevy_ecs::event::EventWriter<model_library::StartPoser>, bevy_ecs::event::EventWriter<creator::upload::StartUpload>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>), fn(bevy_ecs::system::query::Query<(&model_library::LibEntry, &model_library::Checkpoint, core::option::Option<&model_library::Thumbnail>)>, bevy_ecs::change_detection::Res<creator::ui::DefaultTextureId>, bevy_ecs::change_detection::ResMut<creator::library::ShowLibrary>, bevy_ecs::change_detection::ResMut<creator::library::Screen>, bevy_ecs::change_detection::ResMut<comm::Client>), creator::library::browse, creator::library::select, creator::library::wait, creator::library::upload, creator::library::import_model>::{{closure}}

Using bevy_utils::get_short_name in that panic! reduces it down to:

show_library<fn(ResMut<Screen>, Query<(Entity, LibEntry, Option<Thumbnail>, Option<ProcessedData>, Has<PoserTask>)>, Res<DefaultTextureId>, ResMut<ShowLibrary>), fn(ResMut<Screen>, ResMut<ShowLibrary>, Query<(Entity, LibEntry, Option<Thumbnail>, Option<Checkpoint>, Has<TrainingTask>, Option<TrainingPort>)>, Res<DefaultTextureId>, EventWriter<StartTraining>, EventWriter<TryConnect>), fn(ResMut<Screen>, ResMut<ShowLibrary>, Query<TrainingPort, Added<TrainingPort>>, EventWriter<TryConnect>), fn(ResMut<Screen>, Res<Input<KeyCode>>, ResMut<DroppedFile>, Local<PromptState>, EventWriter<StartPoser>, EventWriter<StartUpload>, ResMut<ShowLibrary>, Res<DefaultTextureId>), fn(Query<(LibEntry, Checkpoint, Option<Thumbnail>)>, Res<DefaultTextureId>, ResMut<ShowLibrary>, ResMut<Screen>, ResMut<Client>), browse, select, wait, upload, import_model>::{{closure}}

Would that be an acceptable compromise? Or should a new get_base_name utility be created to just return creator::library::show_library?

[JoJoJet]

Hmm, that's definitely shorter, but I don't think get_short_name is useful here. It strips out the module names, which makes it harder to figure out which system the error is originating from.

[alice-i-cecile]

Yeah I'd prefer a get_base_name solution.

[bushrat011899]

Looks like that should be pretty simple (and mostly allocation free):

/// Strips the name of generic information, leaving only the module path and type name.
pub fn get_base_name(name: &str) -> &str {
    let divider = name
        .find(|c: char| {
            (c == ' ')
                || (c == '<')
                || (c == '>')
                || (c == '(')
                || (c == ')')
                || (c == '[')
                || (c == ']')
                || (c == ',')
                || (c == ';')
        })
        .unwrap_or(name.len());

    name.split_at(divider).0
}

For the above type, this will produce creator::library::show_library as desired. I'm not certain if there are any particular edge cases to consider tho.

[JoJoJet]

I'm not certain if there are any particular edge cases to consider tho.

The best way to find out is to make the PR and let people post the edge cases, haha.

[JoJoJet]

Hmm -- on second thought, this may strip important information from systems with meaningful generic parameters. For example, material_system<ColorMaterial> and material_system<StandardMaterial> would both be normalized into material_system. Is this acceptable?

[bushrat011899]

Hmm -- on second thought, this may strip important information from systems with meaningful generic parameters. For example, material_system<ColorMaterial> and material_system<StandardMaterial> would both be normalized into material_system. Is this acceptable?

That is something that I was worried about. Maybe a combination of get_base_name for a short error description, then a new-line with the get_short_name name for context?

thread 'Compute Task Pool (11)' panicked at 'Resource requested by creator::library::show_library does not exist: comm::Client
System: show_library<fn(ResMut<Screen>, Query<(Entity, LibEntry, Option<Thumbnail>, Option<ProcessedData>, Has<PoserTask>)>, Res<DefaultTextureId>, ResMut<ShowLibrary>), fn(ResMut<Screen>, ResMut<ShowLibrary>, Query<(Entity, LibEntry, Option<Thumbnail>, Option<Checkpoint>, Has<TrainingTask>, Option<TrainingPort>)>, Res<DefaultTextureId>, EventWriter<StartTraining>, EventWriter<TryConnect>), fn(ResMut<Screen>, ResMut<ShowLibrary>, Query<TrainingPort, Added<TrainingPort>>, EventWriter<TryConnect>), fn(ResMut<Screen>, Res<Input<KeyCode>>, ResMut<DroppedFile>, Local<PromptState>, EventWriter<StartPoser>, EventWriter<StartUpload>, ResMut<ShowLibrary>, Res<DefaultTextureId>), fn(Query<(LibEntry, Checkpoint, Option<Thumbnail>)>, Res<DefaultTextureId>, ResMut<ShowLibrary>, ResMut<Screen>, ResMut<Client>), browse, select, wait, upload, import_model>::{{closure}}'

[JoJoJet]

What if we only omit the generics when they're really long?

something like creator::library:show_library<...>

[Laocoon7]

I think the error is long because of the complexity of the system, not because of an issue with the panic message. During a panic, I would rather have too much information than too little. If this were to be shortened, I think the only way to shorten the example properly would be to regex out any type information for types from the bevy::prelude And I still thing that may be problematic for some projects, not to mention a nightmare to keep updated.

I think that any filtering that happens should have a verbose option available to retain as much information as possible. This would require keeping the current implementation behind a feature flag. It doesn't really fall into trace so then possibly another feature flag? Adding a feature flag for verbose_error_messages seems unreasonable.

TLDR: "way too long" is subjective and tied directly to system complexity making this is a non-issue.