instead of manually trying to sync disconnect metrics we now pass a Reason to the handle_disconnect.

There we make sure that the disconnect only happens once and decrease the respective Metrics.
Also added more reasons

common/src/comp/mod.rs

@@ -84,6 +84,7 @@ pub use self::{
         Collider, Density, ForceUpdate, Mass, PhysicsState, Pos, PosVelDefer, PreviousPhysCache,
         Scale, Sticky, Vel,
     },
+    player::DisconnectReason,
     player::Player,
     poise::{Poise, PoiseChange, PoiseSource, PoiseState},
     projectile::{Projectile, ProjectileConstructor},

common/src/comp/player.rs

@@ -5,6 +5,15 @@ use uuid::Uuid;
 
 const MAX_ALIAS_LEN: usize = 32;
 
+#[derive(Debug)]
+pub enum DisconnectReason {
+    Kicked,
+    NewerLogin,
+    NetworkError,
+    Timeout,
+    ClientRequested,
+}
+
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct Player {
     pub alias: String,

common/src/event.rs

@@ -4,7 +4,7 @@ use crate::{
         self,
         invite::{InviteKind, InviteResponse},
         item::Item,
-        Ori, Pos,
+        DisconnectReason, Ori, Pos,
     },
     outcome::Outcome,
     rtsim::RtSimEntity,
@@ -135,7 +135,7 @@ pub enum ServerEvent {
         rtsim_entity: Option<RtSimEntity>,
     },
     CreateWaypoint(Vec3<f32>),
-    ClientDisconnect(EcsEntity),
+    ClientDisconnect(EcsEntity, DisconnectReason),
     ClientDisconnectWithoutPersistence(EcsEntity),
     ChunkRequest(EcsEntity, Vec2<i32>),
     ChatCmd(EcsEntity, String),

server/src/client.rs

@@ -16,7 +16,6 @@ pub struct Client {
     pub participant: Option<Participant>,
     pub last_ping: Mutex<f64>,
     pub login_msg_sent: AtomicBool,
-    pub terminate_msg_recv: AtomicBool,
 
     //TODO: improve network crate so that `send` is no longer `&mut self` and we can get rid of
     // this Mutex. This Mutex is just to please the compiler as we do not get into contention
@@ -68,7 +67,6 @@ impl Client {
             participant: Some(participant),
             last_ping: Mutex::new(last_ping),
             login_msg_sent: AtomicBool::new(false),
-            terminate_msg_recv: AtomicBool::new(false),
             general_stream: Mutex::new(general_stream),
             ping_stream: Mutex::new(ping_stream),
             register_stream: Mutex::new(register_stream),

server/src/cmd.rs

@@ -2477,7 +2477,10 @@ fn kick_player(
     server
         .state
         .mut_resource::<EventBus<ServerEvent>>()
-        .emit_now(ServerEvent::ClientDisconnect(target_player));
+        .emit_now(ServerEvent::ClientDisconnect(
+            target_player,
+            common::comp::DisconnectReason::Kicked,
+        ));
     Ok(())
 }
 

server/src/events/mod.rs

@@ -172,11 +172,16 @@ impl Server {
                     rtsim_entity,
                 } => handle_create_ship(self, pos, ship, mountable, agent, rtsim_entity),
                 ServerEvent::CreateWaypoint(pos) => handle_create_waypoint(self, pos),
-                ServerEvent::ClientDisconnect(entity) => {
+                ServerEvent::ClientDisconnect(entity, reason) => {
-                    frontend_events.push(handle_client_disconnect(self, entity, false))
+                    frontend_events.push(handle_client_disconnect(self, entity, reason, false))
                 },
                 ServerEvent::ClientDisconnectWithoutPersistence(entity) => {
-                    frontend_events.push(handle_client_disconnect(self, entity, true))
+                    frontend_events.push(handle_client_disconnect(
+                        self,
+                        entity,
+                        common::comp::DisconnectReason::Kicked,
+                        true,
+                    ))
                 },
 
                 ServerEvent::ChunkRequest(entity, key) => {

server/src/events/player.rs

@@ -1,7 +1,7 @@
 use super::Event;
 use crate::{
-    client::Client, persistence::character_updater::CharacterUpdater, presence::Presence,
+    client::Client, metrics::PlayerMetrics, persistence::character_updater::CharacterUpdater,
-    state_ext::StateExt, Server,
+    presence::Presence, state_ext::StateExt, Server,
 };
 use common::{
     comp,
@@ -96,9 +96,20 @@ pub fn handle_exit_ingame(server: &mut Server, entity: EcsEntity) {
     }
 }
 
+fn get_reason_str(reason: &comp::DisconnectReason) -> &str {
+    match reason {
+        comp::DisconnectReason::Timeout => "timeout",
+        comp::DisconnectReason::NetworkError => "network_error",
+        comp::DisconnectReason::NewerLogin => "newer_login",
+        comp::DisconnectReason::Kicked => "kicked",
+        comp::DisconnectReason::ClientRequested => "client_requested",
+    }
+}
+
 pub fn handle_client_disconnect(
     server: &mut Server,
     mut entity: EcsEntity,
+    reason: comp::DisconnectReason,
     skip_persistence: bool,
 ) -> Event {
     span!(_guard, "handle_client_disconnect");
@@ -112,6 +123,14 @@ pub fn handle_client_disconnect(
         // receiving multiple `ServerEvent::ClientDisconnect` messages in a tick
         // intended for the same player, so the `None` case here is *not* a bug
         // and we should not log it as a potential issue.
+        server
+            .state()
+            .ecs()
+            .read_resource::<PlayerMetrics>()
+            .clients_disconnected
+            .with_label_values(&[get_reason_str(&reason)])
+            .inc();
+
         if let Some(participant) = client.participant.take() {
             let pid = participant.remote_pid();
             server.runtime.spawn(
@@ -135,7 +154,8 @@ pub fn handle_client_disconnect(
                 .instrument(tracing::debug_span!(
                     "client_disconnect",
                     ?pid,
-                    ?entity
+                    ?entity,
+                    ?reason,
                 )),
             );
         }

server/src/lib.rs

@@ -861,7 +861,7 @@ impl Server {
             for (_, entity) in (&clients, &entities).join() {
                 info!("Emitting client disconnect event for entity: {:?}", entity);
                 let event = if with_persistence {
-                    ServerEvent::ClientDisconnect(entity)
+                    ServerEvent::ClientDisconnect(entity, comp::DisconnectReason::Kicked)
                 } else {
                     ServerEvent::ClientDisconnectWithoutPersistence(entity)
                 };

server/src/sys/msg/general.rs

@@ -1,4 +1,4 @@
-use crate::{client::Client, metrics::PlayerMetrics};
+use crate::client::Client;
 use common::{
     comp::{ChatMode, Player},
     event::{EventBus, ServerEvent},
@@ -9,8 +9,7 @@ use common_ecs::{Job, Origin, Phase, System};
 use common_net::msg::{
     validate_chat_msg, ChatMsgValidationError, ClientGeneral, MAX_BYTES_CHAT_MSG,
 };
-use specs::{Entities, Join, Read, ReadExpect, ReadStorage};
+use specs::{Entities, Join, Read, ReadStorage};
-use std::sync::atomic::Ordering;
 use tracing::{debug, error, warn};
 
 impl Sys {
@@ -19,9 +18,8 @@ impl Sys {
     fn handle_general_msg(
         server_emitter: &mut common::event::Emitter<'_, ServerEvent>,
         entity: specs::Entity,
-        client: &Client,
+        _client: &Client,
         player: Option<&Player>,
-        player_metrics: &ReadExpect<'_, PlayerMetrics>,
         uids: &ReadStorage<'_, Uid>,
         chat_modes: &ReadStorage<'_, ChatMode>,
         msg: ClientGeneral,
@@ -56,12 +54,10 @@ impl Sys {
             },
             ClientGeneral::Terminate => {
                 debug!(?entity, "Client send message to terminate session");
-                player_metrics
+                server_emitter.emit(ServerEvent::ClientDisconnect(
-                    .clients_disconnected
+                    entity,
-                    .with_label_values(&["gracefully"])
+                    common::comp::DisconnectReason::ClientRequested,
-                    .inc();
+                ));
-                client.terminate_msg_recv.store(true, Ordering::Relaxed);
-                server_emitter.emit(ServerEvent::ClientDisconnect(entity));
             },
             _ => unreachable!("not a client_general msg"),
         }
@@ -78,7 +74,6 @@ impl<'a> System<'a> for Sys {
         Entities<'a>,
         Read<'a, EventBus<ServerEvent>>,
         Read<'a, Time>,
-        ReadExpect<'a, PlayerMetrics>,
         ReadStorage<'a, Uid>,
         ReadStorage<'a, ChatMode>,
         ReadStorage<'a, Player>,
@@ -91,16 +86,7 @@ impl<'a> System<'a> for Sys {
 
     fn run(
         _job: &mut Job<Self>,
-        (
+        (entities, server_event_bus, time, uids, chat_modes, players, clients): Self::SystemData,
-            entities,
-            server_event_bus,
-            time,
-            player_metrics,
-            uids,
-            chat_modes,
-            players,
-            clients,
-        ): Self::SystemData,
     ) {
         let mut server_emitter = server_event_bus.emitter();
 
@@ -111,7 +97,6 @@ impl<'a> System<'a> for Sys {
                     entity,
                     client,
                     player,
-                    &player_metrics,
                     &uids,
                     &chat_modes,
                     msg,

server/src/sys/msg/ping.rs

@@ -1,12 +1,11 @@
-use crate::{client::Client, metrics::PlayerMetrics, Settings};
+use crate::{client::Client, Settings};
 use common::{
     event::{EventBus, ServerEvent},
     resources::Time,
 };
 use common_ecs::{Job, Origin, Phase, System};
 use common_net::msg::PingMsg;
-use specs::{Entities, Join, Read, ReadExpect, ReadStorage};
+use specs::{Entities, Join, Read, ReadStorage};
-use std::sync::atomic::Ordering;
 use tracing::{debug, info};
 
 impl Sys {
@@ -28,7 +27,6 @@ impl<'a> System<'a> for Sys {
         Entities<'a>,
         Read<'a, EventBus<ServerEvent>>,
         Read<'a, Time>,
-        ReadExpect<'a, PlayerMetrics>,
         ReadStorage<'a, Client>,
         Read<'a, Settings>,
     );
@@ -39,7 +37,7 @@ impl<'a> System<'a> for Sys {
 
     fn run(
         _job: &mut Job<Self>,
-        (entities, server_event_bus, time, player_metrics, clients, settings): Self::SystemData,
+        (entities, server_event_bus, time, clients, settings): Self::SystemData,
     ) {
         let mut server_emitter = server_event_bus.emitter();
 
@@ -48,14 +46,11 @@ impl<'a> System<'a> for Sys {
 
             match res {
                 Err(e) => {
-                    if !client.terminate_msg_recv.load(Ordering::Relaxed) {
+                    debug!(?entity, ?e, "network error with client, disconnecting");
-                        debug!(?entity, ?e, "network error with client, disconnecting");
+                    server_emitter.emit(ServerEvent::ClientDisconnect(
-                        player_metrics
+                        entity,
-                            .clients_disconnected
+                        common::comp::DisconnectReason::NetworkError,
-                            .with_label_values(&["network_error"])
+                    ));
-                            .inc();
-                        server_emitter.emit(ServerEvent::ClientDisconnect(entity));
-                    }
                 },
                 Ok(1_u64..=u64::MAX) => {
                     // Update client ping.
@@ -66,14 +61,11 @@ impl<'a> System<'a> for Sys {
                     if time.0 - last_ping > settings.client_timeout.as_secs() as f64
                     // Timeout
                     {
-                        if !client.terminate_msg_recv.load(Ordering::Relaxed) {
+                        info!(?entity, "timeout error with client, disconnecting");
-                            info!(?entity, "timeout error with client, disconnecting");
+                        server_emitter.emit(ServerEvent::ClientDisconnect(
-                            player_metrics
+                            entity,
-                                .clients_disconnected
+                            common::comp::DisconnectReason::Timeout,
-                                .with_label_values(&["timeout"])
+                        ));
-                                .inc();
-                            server_emitter.emit(ServerEvent::ClientDisconnect(entity));
-                        }
                     } else if time.0 - last_ping > settings.client_timeout.as_secs() as f64 * 0.5 {
                         // Try pinging the client if the timeout is nearing.
                         client.send_fallible(PingMsg::Ping);

server/src/sys/msg/register.rs

@@ -145,7 +145,10 @@ impl<'a> System<'a> for Sys {
                     .find(|(_, _, old_player)| old_player.uuid() == uuid)
                 {
                     // Remove old client
-                    server_event_bus.emit_now(ServerEvent::ClientDisconnect(old_entity));
+                    server_event_bus.emit_now(ServerEvent::ClientDisconnect(
+                        old_entity,
+                        common::comp::DisconnectReason::NewerLogin,
+                    ));
                     let _ = old_client.send(ServerGeneral::Disconnect(DisconnectReason::Kicked(
                         String::from("You have logged in from another location."),
                     )));