Fix bugged room state from lingering connection

When disconnecting from a room whose instance is "waiting" and then
reconnecting, the old instance would not be stopped immediately.
Instead, it would continue to run until it managed to reconnect, sending
status updates to the main event bus in the process.

These events led to the euph::Room entering a state where it was
connected but no last_msg_id was set. This meant that no new messages
could be entered into the vault, including messages sent by the user.
The result was UI weirdness when sending a message.

As a fix, euphoxide instances are now identified via an u32 id. This id
is unique across all rooms. Packets by unknown ids are rejected and have
no effect on room states.

Cargo.lock

@@ -320,7 +320,7 @@ dependencies = [
 [[package]]
 name = "euphoxide"
 version = "0.3.0"
-source = "git+https://github.com/Garmelon/euphoxide.git?tag=v0.3.0#c5be90cd60ff1fac78c2063c812d88dd36858481"
+source = "git+https://github.com/Garmelon/euphoxide.git?rev=069e4e02c77e5bc649cd770af8183d8be8cd52f5#069e4e02c77e5bc649cd770af8183d8be8cd52f5"
 dependencies = [
  "async-trait",
  "caseless",

Cargo.toml

@@ -35,7 +35,7 @@ features = ["rustls-tls-native-roots"]
 
 [dependencies.euphoxide]
 git = "https://github.com/Garmelon/euphoxide.git"
-tag = "v0.3.0"
+rev = "069e4e02c77e5bc649cd770af8183d8be8cd52f5"
 features = ["bot"]
 
 # [patch."https://github.com/Garmelon/euphoxide.git"]

src/euph/room.rs

@@ -85,6 +85,10 @@ impl Room {
         self.instance.stopped()
     }
 
+    pub fn instance(&self) -> &Instance {
+        &self.instance
+    }
+
     pub fn state(&self) -> &State {
         &self.state
     }
@@ -108,7 +112,7 @@ impl Room {
                     self.log_request_canary = Some(tx);
                     let vault_clone = self.vault.clone();
                     let conn_tx_clone = conn_tx.clone();
-                    debug!("{}: spawning log request task", self.instance.config().name);
+                    debug!("{}: spawning log request task", self.instance.config().room);
                     tokio::task::spawn(async move {
                         select! {
                             _ = rx => {},
@@ -204,34 +208,34 @@ impl Room {
     }
 
     async fn on_packet(&mut self, packet: ParsedPacket) {
-        let instance_name = &self.instance.config().name;
+        let room_name = &self.instance.config().room;
         let data = ok_or_return!(&packet.content);
         match data {
             Data::BounceEvent(_) => {}
             Data::DisconnectEvent(_) => {}
             Data::HelloEvent(_) => {}
             Data::JoinEvent(d) => {
-                debug!("{instance_name}: {:?} joined", d.0.name);
+                debug!("{room_name}: {:?} joined", d.0.name);
             }
             Data::LoginEvent(_) => {}
             Data::LogoutEvent(_) => {}
             Data::NetworkEvent(d) => {
-                warn!("{instance_name}: network event ({})", d.r#type);
+                warn!("{room_name}: network event ({})", d.r#type);
             }
             Data::NickEvent(d) => {
-                debug!("{instance_name}: {:?} renamed to {:?}", d.from, d.to);
+                debug!("{room_name}: {:?} renamed to {:?}", d.from, d.to);
             }
             Data::EditMessageEvent(_) => {
-                info!("{instance_name}: a message was edited");
+                info!("{room_name}: a message was edited");
             }
             Data::PartEvent(d) => {
-                debug!("{instance_name}: {:?} left", d.0.name);
+                debug!("{room_name}: {:?} left", d.0.name);
             }
             Data::PingEvent(_) => {}
             Data::PmInitiateEvent(d) => {
                 // TODO Show info popup and automatically join PM room
                 info!(
-                    "{instance_name}: {:?} initiated a pm from &{}",
+                    "{room_name}: {:?} initiated a pm from &{}",
                     d.from_nick, d.from_room
                 );
             }
@@ -247,7 +251,7 @@ impl Room {
                 }
             }
             Data::SnapshotEvent(d) => {
-                info!("{instance_name}: successfully joined");
+                info!("{room_name}: successfully joined");
                 logging_unwrap!(self.vault.join(Time::now()).await);
                 self.last_msg_id = Some(d.log.last().map(|m| m.id));
                 logging_unwrap!(

src/ui/euph/room.rs

@@ -95,15 +95,16 @@ impl EuphRoom {
         self.vault().room()
     }
 
-    pub fn connect(&mut self) {
+    pub fn connect(&mut self, next_instance_id: &mut u32) {
         if self.room.is_none() {
             let instance_config = self
                 .server_config
                 .clone()
-                .room(self.vault().room().to_string())
+                .room(*next_instance_id, self.vault().room().to_string())
                 .username(self.config.username.clone())
                 .force_username(self.config.force_username)
                 .password(self.config.password.clone());
+            *next_instance_id = next_instance_id.wrapping_add(1);
 
             let tx = self.ui_event_tx.clone();
             self.room = Some(euph::Room::new(
@@ -674,22 +675,35 @@ impl EuphRoom {
     }
 
     pub async fn handle_event(&mut self, event: Event) -> bool {
-        let handled = if self.room.is_some() {
+        let room = match &self.room {
-            if let Event::Packet(_, packet, _) = &event {
+            None => return false,
+            Some(room) => room,
+        };
+
+        if event.config().id != room.instance().config().id {
+            // If we allowed ids other than the current one, old instances that
+            // haven't yet shut down properly could mess up our state.
+            return false;
+        }
+
+        // We handle the packet internally first because the room event handling
+        // will consume it while we only need a reference.
+        let handled = if let Event::Packet(_, packet, _) = &event {
             match &packet.content {
                 Ok(data) => self.handle_euph_data(data),
                 Err(reason) => self.handle_euph_error(packet.r#type, reason),
             }
         } else {
+            // The room state changes, which always means a redraw.
             true
-            }
-        } else {
-            false
         };
 
-        if let Some(room) = &mut self.room {
+        self.room
-            room.handle_event(event).await;
+            .as_mut()
-        }
+            // See check at the beginning of the function.
+            .expect("no room even though we checked earlier")
+            .handle_event(event)
+            .await;
 
         handled
     }

src/ui/rooms.rs

@@ -61,6 +61,7 @@ pub struct Rooms {
     order: Order,
 
     euph_server_config: ServerConfig,
+    euph_next_instance_id: u32,
     euph_rooms: HashMap<String, EuphRoom>,
 }
 
@@ -81,13 +82,14 @@ impl Rooms {
             list: ListState::new(),
             order: Order::from_rooms_sort_order(config.rooms_sort_order),
             euph_server_config,
+            euph_next_instance_id: 0,
             euph_rooms: HashMap::new(),
         };
 
         if !config.offline {
             for (name, config) in &config.euph.rooms {
                 if config.autojoin {
-                    result.get_or_insert_room(name.clone()).connect();
+                    result.connect_to_room(name.clone());
                 }
             }
         }
@@ -106,6 +108,36 @@ impl Rooms {
         })
     }
 
+    fn connect_to_room(&mut self, name: String) {
+        let room = self.euph_rooms.entry(name.clone()).or_insert_with(|| {
+            EuphRoom::new(
+                self.euph_server_config.clone(),
+                self.config.euph_room(&name),
+                self.vault.euph().room(name),
+                self.ui_event_tx.clone(),
+            )
+        });
+        room.connect(&mut self.euph_next_instance_id);
+    }
+
+    fn connect_to_all_rooms(&mut self) {
+        for room in self.euph_rooms.values_mut() {
+            room.connect(&mut self.euph_next_instance_id);
+        }
+    }
+
+    fn disconnect_from_room(&mut self, name: &str) {
+        if let Some(room) = self.euph_rooms.get_mut(name) {
+            room.disconnect();
+        }
+    }
+
+    fn disconnect_from_all_rooms(&mut self) {
+        for room in self.euph_rooms.values_mut() {
+            room.disconnect();
+        }
+    }
+
     /// Remove rooms that are not running any more and can't be found in the db.
     /// Insert rooms that are in the db but not yet in in the hash map.
     ///
@@ -370,36 +402,28 @@ impl Rooms {
             }
             key!('c') => {
                 if let Some(name) = self.list.cursor() {
-                    if let Some(room) = self.euph_rooms.get_mut(&name) {
+                    self.connect_to_room(name);
-                        room.connect();
-                    }
                 }
                 return true;
             }
             key!('C') => {
-                for room in self.euph_rooms.values_mut() {
+                self.connect_to_all_rooms();
-                    room.connect();
-                }
                 return true;
             }
             key!('d') => {
                 if let Some(name) = self.list.cursor() {
-                    if let Some(room) = self.euph_rooms.get_mut(&name) {
+                    self.disconnect_from_room(&name);
-                        room.disconnect();
-                    }
                 }
                 return true;
             }
             key!('D') => {
-                for room in self.euph_rooms.values_mut() {
+                self.disconnect_from_all_rooms();
-                    room.disconnect();
-                }
                 return true;
             }
             key!('a') => {
                 for (name, options) in &self.config.euph.rooms {
                     if options.autojoin {
-                        self.get_or_insert_room(name.clone()).connect();
+                        self.connect_to_room(name.clone());
                     }
                 }
                 return true;
@@ -511,7 +535,7 @@ impl Rooms {
                 key!(Enter) => {
                     let name = ed.text();
                     if !name.is_empty() {
-                        self.get_or_insert_room(name.clone()).connect();
+                        self.connect_to_room(name.clone());
                         self.state = State::ShowRoom(name);
                     }
                     return true;
@@ -545,12 +569,13 @@ impl Rooms {
     }
 
     pub async fn handle_euph_event(&mut self, event: Event) -> bool {
-        let instance_name = event.config().name.clone();
+        let room_name = event.config().room.clone();
-        let room = self.get_or_insert_room(instance_name.clone());
+        let Some(room) = self.euph_rooms.get_mut(&room_name) else { return false; };
+
         let handled = room.handle_event(event).await;
 
         let room_visible = match &self.state {
-            State::ShowRoom(name) => *name == instance_name,
+            State::ShowRoom(name) => *name == room_name,
             _ => true,
         };
         handled && room_visible