Fix span macros by putting cfgs outside the macro (they are evaluated in the crate where the macro is used), add shorthand for common case of prof_span macro, add some spans to the client code and spiff bits of it

client/src/lib.rs

@@ -43,7 +43,7 @@ use common::{
     uid::{Uid, UidAllocator},
     vol::RectVolSize,
 };
-use common_base::span;
+use common_base::{prof_span, span};
 use common_net::{
     msg::{
         self, validate_chat_msg,
@@ -760,6 +760,7 @@ impl Client {
     where
         S: Into<ClientMsg>,
     {
+        prof_span!("send_msg_err");
         let msg: ClientMsg = msg.into();
         #[cfg(debug_assertions)]
         {
@@ -1426,6 +1427,7 @@ impl Client {
         // 1) Handle input from frontend.
         // Pass character actions from frontend input to the player's entity.
         if self.presence.is_some() {
+            prof_span!("handle and send inputs");
             if let Err(e) = self
                 .state
                 .ecs()
@@ -1457,21 +1459,25 @@ impl Client {
 
         // Prepare for new events
         {
+            prof_span!("Last<CharacterState> comps update");
             let ecs = self.state.ecs();
-            for (entity, _) in (&ecs.entities(), &ecs.read_storage::<comp::Body>()).join() {
-                let mut last_character_states =
-                    ecs.write_storage::<comp::Last<comp::CharacterState>>();
-                if let Some(client_character_state) =
-                    ecs.read_storage::<comp::CharacterState>().get(entity)
+            let mut last_character_states = ecs.write_storage::<comp::Last<comp::CharacterState>>();
+            for (entity, _, character_state) in (
+                &ecs.entities(),
+                &ecs.read_storage::<comp::Body>(),
+                &ecs.read_storage::<comp::CharacterState>(),
+            )
+                .join()
+            {
+                if let Some(l) = last_character_states
+                    .entry(entity)
+                    .ok()
+                    .map(|l| l.or_insert_with(|| comp::Last(character_state.clone())))
+                    // TODO: since this just updates when the variant changes we should
+                    // just store the variant to avoid the clone overhead
+                    .filter(|l| !character_state.same_variant(&l.0))
                 {
-                    if let Some(l) = last_character_states
-                        .entry(entity)
-                        .ok()
-                        .map(|l| l.or_insert_with(|| comp::Last(client_character_state.clone())))
-                        .filter(|l| !client_character_state.same_variant(&l.0))
-                    {
-                        *l = comp::Last(client_character_state.clone());
-                    }
+                    *l = comp::Last(character_state.clone());
                 }
             }
         }
@@ -1520,6 +1526,7 @@ impl Client {
             .get(self.entity())
             .cloned();
         if let (Some(pos), Some(view_distance)) = (pos, self.view_distance) {
+            prof_span!("terrain");
             let chunk_pos = self.state.terrain().pos_key(pos.0.map(|e| e as i32));
 
             // Remove chunks that are too far from the player.
@@ -1653,6 +1660,7 @@ impl Client {
         frontend_events: &mut Vec<Event>,
         msg: ServerGeneral,
     ) -> Result<(), Error> {
+        prof_span!("handle_server_msg");
         match msg {
             ServerGeneral::Disconnect(reason) => match reason {
                 DisconnectReason::Shutdown => return Err(Error::ServerShutdown),
@@ -1805,6 +1813,7 @@ impl Client {
         frontend_events: &mut Vec<Event>,
         msg: ServerGeneral,
     ) -> Result<(), Error> {
+        prof_span!("handle_server_in_game_msg");
         match msg {
             ServerGeneral::GroupUpdate(change_notification) => {
                 use comp::group::ChangeNotification::*;
@@ -1979,6 +1988,7 @@ impl Client {
 
     #[allow(clippy::unnecessary_wraps)]
     fn handle_server_terrain_msg(&mut self, msg: ServerGeneral) -> Result<(), Error> {
+        prof_span!("handle_server_terrain_mgs");
         match msg {
             ServerGeneral::TerrainChunkUpdate { key, chunk } => {
                 if let Some(chunk) = chunk.ok().and_then(|c| c.to_chunk()) {
@@ -2004,6 +2014,7 @@ impl Client {
         events: &mut Vec<Event>,
         msg: ServerGeneral,
     ) -> Result<(), Error> {
+        prof_span!("handle_character_screen_msg");
         match msg {
             ServerGeneral::CharacterListUpdate(character_list) => {
                 self.character_list.characters = character_list;
@@ -2034,6 +2045,7 @@ impl Client {
     }
 
     fn handle_ping_msg(&mut self, msg: PingMsg) -> Result<(), Error> {
+        prof_span!("handle_ping_msg");
         match msg {
             PingMsg::Ping => {
                 self.send_msg_err(PingMsg::Pong)?;
@@ -2088,6 +2100,7 @@ impl Client {
 
     /// Handle new server messages.
     fn handle_new_messages(&mut self) -> Result<Vec<Event>, Error> {
+        prof_span!("handle_new_messages");
         let mut frontend_events = Vec::new();
 
         // Check that we have an valid connection.

common/base/src/lib.rs

@@ -6,20 +6,21 @@ pub use userdata_dir::userdata_dir;
 
 #[cfg(feature = "tracy")] pub use tracy_client;
 
+#[cfg(not(feature = "tracy"))]
 #[macro_export]
 macro_rules! plot {
     ($name:expr, $value:expr) => {
-        #[cfg(feature = "tracy")]
-        {
-            use $crate::tracy_client::{create_plot, Plot};
-            static PLOT: Plot = create_plot!($name);
-            PLOT.point($value);
-        }
-        #[cfg(not(feature = "tracy"))]
-        {
-            // type check
-            let _: f64 = $value;
-        }
+        // type check
+        let _: f64 = $value;
+    };
+}
+#[cfg(feature = "tracy")]
+#[macro_export]
+macro_rules! plot {
+    ($name:expr, $value:expr) => {
+        use $crate::tracy_client::{create_plot, Plot};
+        static PLOT: Plot = create_plot!($name);
+        PLOT.point($value);
     };
 }
 
@@ -45,6 +46,7 @@ macro_rules! dev_panic {
 }
 
 // https://discordapp.com/channels/676678179678715904/676685797524766720/723358438943621151
+#[cfg(not(feature = "tracy"))]
 #[macro_export]
 macro_rules! span {
     ($guard_name:tt, $level:ident, $name:expr, $($fields:tt)*) => {
@@ -56,12 +58,27 @@ macro_rules! span {
         let $guard_name = span.enter();
     };
     ($guard_name:tt, $name:expr) => {
-        #[cfg(not(feature = "tracy"))]
         let span = tracing::span!(tracing::Level::TRACE, $name);
-        #[cfg(not(feature = "tracy"))]
         let $guard_name = span.enter();
+    };
+    ($guard_name:tt, $no_tracy_name:expr, $tracy_name:expr) => {
+        $crate::span!($guard_name, $no_tracy_name);
+    };
+}
+
+#[cfg(feature = "tracy")]
+#[macro_export]
+macro_rules! span {
+    ($guard_name:tt, $level:ident, $name:expr, $($fields:tt)*) => {
+        let span = tracing::span!(tracing::Level::$level, $name, $($fields)*);
+        let $guard_name = span.enter();
+    };
+    ($guard_name:tt, $level:ident, $name:expr) => {
+        let span = tracing::span!(tracing::Level::$level, $name);
+        let $guard_name = span.enter();
+    };
+    ($guard_name:tt, $name:expr) => {
         // Directly use `tracy_client` to decrease overhead for better timing
-        #[cfg(feature = "tracy")]
         let $guard_name = $crate::tracy_client::Span::new(
             $name,
             "",
@@ -72,9 +89,6 @@ macro_rules! span {
         );
     };
     ($guard_name:tt, $no_tracy_name:expr, $tracy_name:expr) => {
-        #[cfg(not(feature = "tracy"))]
-        $crate::span!($guard_name, $no_tracy_name);
-        #[cfg(feature = "tracy")]
         $crate::span!($guard_name, $tracy_name);
     };
 }
@@ -87,9 +101,22 @@ pub struct ProfSpan;
 /// Like the span macro but only used when profiling and not in regular tracing
 /// operations
 #[macro_export]
+#[cfg(not(feature = "tracy"))]
+macro_rules! prof_span {
+    ($guard_name:tt, $name:expr) => {
+        let $guard_name = $crate::ProfSpan;
+    };
+    // Shorthand for when you want the guard to just be dropped at the end of the scope instead
+    // of controlling it manually
+    ($name:expr) => {};
+}
+
+/// Like the span macro but only used when profiling and not in regular tracing
+/// operations
+#[macro_export]
+#[cfg(feature = "tracy")]
 macro_rules! prof_span {
     ($guard_name:tt, $name:expr) => {
-        #[cfg(feature = "tracy")]
         let $guard_name = $crate::ProfSpan($crate::tracy_client::Span::new(
             $name,
             "",
@@ -98,8 +125,11 @@ macro_rules! prof_span {
             // No callstack since this has significant overhead
             0,
         ));
-        #[cfg(not(feature = "tracy"))]
-        let $guard_name = $crate::ProfSpan;
+    };
+    // Shorthand for when you want the guard to just be dropped at the end of the scope instead
+    // of controlling it manually
+    ($name:expr) => {
+        $crate::prof_span!(_guard, $name);
     };
 }