fix: download AllMaps by TCP to prevent issues with large data over UDP (#85)

Author: pbellchambers

CHANGELOG.md

@@ -36,7 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - fix players not disappearing when disconnecting
 - combat can no longer occur after player has moved away from target
 - fix potential server crash when calculating loop sleep duration
-- temporary fix - retry mechanism for client map download from server on unreliable connections
+- client map download now via tcp to avoid issues with large data over unreliable connections on udp
 
 ### 🧰 Maintenance
 - updated dependencies

Cargo.lock

@@ -26,5 +26,5 @@ serde = { version = "1.0.142", features = ["derive"] }
 bincode = "1.3.3"
 regex = "1.6.0"
 chrono = "0.4.20"
-itertools = "0.10.3"
 rayon = "1.5.3"
+message-io = { version = "0.14.6", default-features = false, features = ["tcp"] }

rustyhack_client/Cargo.toml

@@ -25,6 +25,7 @@ pub(super) fn run(
     sender: &Sender<Packet>,
     receiver: Receiver<SocketEvent>,
     server_addr: &str,
+    server_tcp_addr: &str,
     client_addr: &str,
     player_name: &str,
 ) {
@@ -37,13 +38,7 @@ pub(super) fn run(
     );
 
     //get basic data from server needed to start client_game
-    //todo fix properly - repeating the request when it fails is a temporary fix
-    let mut all_maps_option = None;
-    while all_maps_option.is_none() {
-        all_maps_option =
-            map_downloader::request_all_maps_data(sender, server_addr, &player_update_receiver);
-    }
-    let all_maps = all_maps_option.unwrap();
+    let all_maps = map_downloader::request_all_maps_data(server_tcp_addr);
 
     //create player
     let mut player = new_player::send_new_player_request(

rustyhack_client/src/client_game.rs

@@ -1,96 +1,87 @@
 use bincode::{deserialize, serialize};
-use crossbeam_channel::{Receiver, Sender};
-use itertools::Itertools;
-use laminar::Packet;
+use message_io::network::{NetEvent, Transport};
+use message_io::node;
+use message_io::node::NodeEvent;
 use rustyhack_lib::background_map::AllMaps;
 use rustyhack_lib::network::packets::{PlayerRequest, ServerMessage};
 use std::collections::HashMap;
-use std::thread;
-use std::time::Duration;
+use std::process;
 
-pub(crate) fn request_all_maps_data(
-    sender: &Sender<Packet>,
-    server_addr: &str,
-    channel_receiver: &Receiver<ServerMessage>,
-) -> Option<AllMaps> {
-    let get_all_maps_request_packet = Packet::reliable_ordered(
-        server_addr
-            .parse()
-            .expect("Server address format is invalid."),
-        serialize(&PlayerRequest::GetChunkedAllMaps)
-            .expect("Error serializing GetAllMaps request."),
-        Some(1),
-    );
-    rustyhack_lib::network::send_packet(get_all_maps_request_packet, sender);
-    info!("Requested all maps data from server.");
-    wait_for_all_maps_response(channel_receiver)
-}
+pub(crate) fn request_all_maps_data(server_tcp_addr: &str) -> AllMaps {
+    let mut all_maps = HashMap::new();
+    let (handler, listener) = node::split();
 
-fn wait_for_all_maps_response(channel_receiver: &Receiver<ServerMessage>) -> Option<AllMaps> {
-    let mut all_maps_downloaded = false;
-    let mut all_maps = None;
-    let mut all_maps_chunks = HashMap::new();
-    loop {
-        let received = channel_receiver.recv();
-        if let Ok(received_message) = received {
-            match received_message {
-                ServerMessage::AllMapsChunk(message) => {
-                    info!("All maps chunk received from server: {}", message.0);
-                    all_maps_chunks.insert(message.0, message.1);
-                }
-                ServerMessage::AllMapsChunksComplete => {
-                    info!("All maps chunks downloaded from server.");
-                    all_maps = combine_all_maps_chunks(&all_maps_chunks);
-                    all_maps_downloaded = true;
-                }
-                _ => {
-                    info!(
-                        "Ignoring other message types until maps downloaded. {:?}",
-                        received_message
-                    );
+    //connect to server tcp port
+    let (server, _) = handler
+        .network()
+        .connect(Transport::FramedTcp, server_tcp_addr)
+        .unwrap_or_else(|err| {
+            error!(
+                "Unable to connect to server {}, error: {}",
+                &server_tcp_addr, err
+            );
+            process::exit(1);
+        });
+
+    //send GetAllMaps request and wait for response
+    listener.for_each(|event| match event {
+        NodeEvent::Network(net_event) => match net_event {
+            NetEvent::Connected(_endpoint, _ok) => {
+                info!("Sending GetAllMaps request to server.");
+                handler.signals().send(PlayerRequest::GetAllMaps);
+            }
+            NetEvent::Accepted(_, _) => unreachable!(), // Only generated by listening
+            NetEvent::Message(_endpoint, data) => {
+                info!("Received raw AllMaps data from server.");
+                let all_maps_option = deserialize_all_maps_reply(data);
+                match all_maps_option {
+                    None => {
+                        info!("Sending GetAllMaps request to server.");
+                        handler.signals().send(PlayerRequest::GetAllMaps);
+                    }
+                    Some(all_maps_data) => {
+                        all_maps = all_maps_data;
+                        handler.stop();
+                        handler.network().remove(server.resource_id());
+                    }
                 }
             }
-        }
-        if all_maps_downloaded {
-            info!("Got all maps data.");
-            break;
-        }
-        thread::sleep(Duration::from_millis(1));
-    }
-    debug!("All maps is: {:?}", all_maps);
+            NetEvent::Disconnected(_endpoint) => {
+                error!("Server is disconnected.");
+                handler.stop();
+                handler.network().remove(server.resource_id());
+                process::exit(1);
+            }
+        },
+        NodeEvent::Signal(signal) => match signal {
+            PlayerRequest::GetAllMaps => {
+                handler
+                    .network()
+                    .send(server, &serialize(&PlayerRequest::GetAllMaps).unwrap());
+            }
+            _ => {
+                warn!("Ignoring invalid PlayerRequest on tcp channel.");
+            }
+        },
+    });
     all_maps
 }
 
-fn combine_all_maps_chunks(all_maps_chunks: &HashMap<usize, Vec<u8>>) -> Option<AllMaps> {
-    deserialize_all_maps_reply(&combine_chunks(all_maps_chunks))
-}
-
-fn combine_chunks(all_maps_chunks: &HashMap<usize, Vec<u8>>) -> Vec<u8> {
-    let mut combined_all_maps_chunks: Vec<u8> = Vec::new();
-    for chunk in all_maps_chunks.keys().sorted() {
-        combined_all_maps_chunks.extend(
-            all_maps_chunks
-                .get(chunk)
-                .expect("Error combining all maps chunks on chunk."),
-        );
-    }
-    combined_all_maps_chunks
-}
-
-fn deserialize_all_maps_reply(combined_chunks: &[u8]) -> Option<AllMaps> {
-    let deserialized_chunks = deserialize::<ServerMessage>(combined_chunks);
-    match deserialized_chunks {
+fn deserialize_all_maps_reply(data: &[u8]) -> Option<AllMaps> {
+    let deserialized_data = deserialize::<ServerMessage>(data);
+    match deserialized_data {
         Ok(deserialized) => {
             if let ServerMessage::AllMaps(all_maps) = deserialized {
+                info!("AllMaps data downloaded successfully.");
                 Some(all_maps)
             } else {
-                warn!("Deserialized message from server was not all maps, will request again.");
+                warn!("Deserialized message from server was not valid AllMaps data, will request again.");
                 None
             }
         }
         Err(error) => {
-            warn!(
-                "Error deserializing all maps from server, will request again. {}",
+            error!(
+                "Error deserializing AllMaps from server, will request again. {}",
                 error.to_string()
             );
             None