From e61d7920e30c4fd0da43726875eb21b7b53d52eb Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 3 Mar 2021 03:43:15 +0000 Subject: [PATCH] Add suffixes to diagnostics and other cleanup (#1505) Also a few related clean ups to diagnostics Old look (from the log_diagnostics_example):
Old look hidden to avoid clutter ![image](https://user-images.githubusercontent.com/36049421/108776774-bc349080-755a-11eb-8569-d4832abf6bc3.png)
New look: ![image](https://user-images.githubusercontent.com/36049421/108776587-82638a00-755a-11eb-96eb-539026d59bcb.png) In particular, notice that the width of the diagnostics has been significantly reduced - within vscode the value no longer wraps on my 1920 width monitor. The value is still 105 columns wide, so there is room for improvement however. --- .../asset_count_diagnostics_plugin.rs | 2 +- crates/bevy_diagnostic/src/diagnostic.rs | 31 ++++++++++++++++--- .../src/frame_time_diagnostics_plugin.rs | 2 +- .../src/log_diagnostics_plugin.rs | 24 ++++++++++++-- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs b/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs index db3d773d50..fb0bd6c5e6 100644 --- a/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs +++ b/crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs @@ -24,7 +24,7 @@ impl AssetCountDiagnosticsPlugin { pub fn setup_system(mut diagnostics: ResMut) { diagnostics.add(Diagnostic::new( Self::diagnostic_id(), - &format!("asset_count {}", std::any::type_name::()), + format!("asset_count {}", std::any::type_name::()), 20, )); } diff --git a/crates/bevy_diagnostic/src/diagnostic.rs b/crates/bevy_diagnostic/src/diagnostic.rs index 416bb8ac45..97e5d8bcf7 100644 --- a/crates/bevy_diagnostic/src/diagnostic.rs +++ b/crates/bevy_diagnostic/src/diagnostic.rs @@ -1,5 +1,8 @@ +use bevy_log::warn; use bevy_utils::{Duration, Instant, StableHashMap, Uuid}; -use std::collections::VecDeque; +use std::{borrow::Cow, collections::VecDeque}; + +use crate::log_diagnostics_plugin::MAX_LOG_NAME_WIDTH; /// Unique identifier for a [Diagnostic] #[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, PartialOrd, Ord)] @@ -29,7 +32,8 @@ pub struct DiagnosticMeasurement { #[derive(Debug)] pub struct Diagnostic { pub id: DiagnosticId, - pub name: String, + pub name: Cow<'static, str>, + pub suffix: Cow<'static, str>, history: VecDeque, sum: f64, max_history_length: usize, @@ -49,16 +53,35 @@ impl Diagnostic { .push_front(DiagnosticMeasurement { time, value }); } - pub fn new(id: DiagnosticId, name: &str, max_history_length: usize) -> Diagnostic { + pub fn new( + id: DiagnosticId, + name: impl Into>, + max_history_length: usize, + ) -> Diagnostic { + let name = name.into(); + if name.chars().count() > MAX_LOG_NAME_WIDTH { + // This could be a false positive due to a unicode width being shorter + warn!( + "Diagnostic {:?} has name longer than {} characters, and so might overflow in the LogDiagnosticsPlugin\ + Consider using a shorter name.", + name, MAX_LOG_NAME_WIDTH + ) + } Diagnostic { id, - name: name.to_string(), + name, + suffix: Cow::Borrowed(""), history: VecDeque::with_capacity(max_history_length), max_history_length, sum: 0.0, } } + pub fn with_suffix(mut self, suffix: impl Into>) -> Self { + self.suffix = suffix.into(); + self + } + pub fn value(&self) -> Option { self.history.back().map(|measurement| measurement.value) } diff --git a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs index 5eef7c917d..d4dbdb7ed2 100644 --- a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs @@ -27,7 +27,7 @@ impl FrameTimeDiagnosticsPlugin { DiagnosticId::from_u128(73441630925388532774622109383099159699); pub fn setup_system(mut diagnostics: ResMut) { - diagnostics.add(Diagnostic::new(Self::FRAME_TIME, "frame_time", 20)); + diagnostics.add(Diagnostic::new(Self::FRAME_TIME, "frame_time", 20).with_suffix("s")); diagnostics.add(Diagnostic::new(Self::FPS, "fps", 20)); diagnostics.add(Diagnostic::new(Self::FRAME_COUNT, "frame_count", 1)); } diff --git a/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs index 7b3b3b9175..dda1804670 100644 --- a/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs +++ b/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs @@ -28,6 +28,10 @@ impl Default for LogDiagnosticsPlugin { } } +/// The width which diagnostic names will be printed as +/// Plugin names should not be longer than this value +pub(crate) const MAX_LOG_NAME_WIDTH: usize = 32; + impl Plugin for LogDiagnosticsPlugin { fn build(&self, app: &mut bevy_app::AppBuilder) { app.insert_resource(LogDiagnosticsState { @@ -58,11 +62,25 @@ impl LogDiagnosticsPlugin { if let Some(value) = diagnostic.value() { if let Some(average) = diagnostic.average() { info!( - "{:<65}: {:<10.6} (avg {:.6})", - diagnostic.name, value, average + target: "bevy diagnostic", + "{:12} (avg {:>})", + diagnostic.name, + // Suffix is only used for 's' as in seconds currently, + // so we reserve one column for it + format!("{:.6}{:1}", value, diagnostic.suffix), + // Do not reserve one column for the suffix in the average + // The ) hugging the value is more aesthetically pleasing + format!("{:.6}{:}", average, diagnostic.suffix), + name_width = MAX_LOG_NAME_WIDTH, ); } else { - info!("{:<65}: {:<10.6}", diagnostic.name, value); + info!( + target: "bevy diagnostic", + "{:}", + diagnostic.name, + format!("{:.6}{:}", value, diagnostic.suffix), + name_width = MAX_LOG_NAME_WIDTH, + ); } } }