feat(protocols): Add more control over gathering additional information (#180)

* protocols: Add more control over gathering additional information

Adds GatherToggle which allows choosing the behaviour for how the query
handles fetching additional information. The choices are:
- DontGather - Don't attempt to fetch information
- AttemptGather - Try to fetch the information but ignore errors
- Required - Try to fetch information and fail if it errors

A handy macro was also added to utils to dispatch additional queries
based on a GatherToggle value.

* Add/Update badge

* protocols: Improve GatherToggle enum names

Co-Authored-By: Cain <75994858+cainthebest@users.noreply.github.com>
Co-Authored-By: CosminPerRam <cosmin.p@live.com>

* Add/Update badge

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Cain <75994858+cainthebest@users.noreply.github.com>
Co-authored-by: CosminPerRam <cosmin.p@live.com>

crates/lib/examples/valve_protocol_query.rs

@@ -1,3 +1,4 @@
+use gamedig::protocols::types::GatherToggle;
 use gamedig::protocols::valve;
 use gamedig::protocols::valve::{Engine, GatheringSettings};
 use gamedig::TimeoutSettings;
@@ -8,9 +9,9 @@ fn main() {
     let address = &SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 27015);
     let engine = Engine::Source(None); // We don't specify a steam app id, let the query try to find it.
     let gather_settings = GatheringSettings {
-        players: true,       // We want to query for players
+        players: GatherToggle::Enforce, // We want to query for players
-        rules: false,        // We don't want to query for rules
+        rules: GatherToggle::Skip,      // We don't want to query for rules
-        check_app_id: false, // Loosen up the query a bit by not checking app id
+        check_app_id: false,            // Loosen up the query a bit by not checking app id
     };
 
     let read_timeout = Duration::from_secs(2);

crates/lib/src/games/definitions.rs

@@ -4,7 +4,7 @@ use crate::games::minecraft::types::{LegacyGroup, Server};
 use crate::protocols::{gamespy::GameSpyVersion, quake::QuakeVersion, valve::Engine, Protocol};
 use crate::Game;
 
-use crate::protocols::types::ProprietaryProtocol;
+use crate::protocols::types::{GatherToggle, ProprietaryProtocol};
 use crate::protocols::valve::GatheringSettings;
 use phf::{phf_map, Map};
 
@@ -40,8 +40,8 @@ pub static GAMES: Map<&'static str, Game> = phf_map! {
     "minecraftlegacy14" => game!("Minecraft (legacy 1.4)", 25565, Protocol::PROPRIETARY(ProprietaryProtocol::Minecraft(Some(Server::Legacy(LegacyGroup::V1_4))))),
     "minecraftlegacyb18" => game!("Minecraft (legacy b1.8)", 25565, Protocol::PROPRIETARY(ProprietaryProtocol::Minecraft(Some(Server::Legacy(LegacyGroup::VB1_8))))),
     "aapg" => game!("America's Army: Proving Grounds", 27020, Protocol::Valve(Engine::new(203_290)), GatheringSettings {
-        players: true,
+        players: GatherToggle::Enforce,
-        rules: false,
+        rules: GatherToggle::Skip,
         check_app_id: true,
     }.into_extra()),
     "alienswarm" => game!("Alien Swarm", 27015, Protocol::Valve(Engine::new(630))),
@@ -53,8 +53,8 @@ pub static GAMES: Map<&'static str, Game> = phf_map! {
     "avorion" => game!("Avorion", 27020, Protocol::Valve(Engine::new(445_220))),
     "barotrauma" => game!("Barotrauma", 27016, Protocol::Valve(Engine::new(602_960))),
     "basedefense" => game!("Base Defense", 27015, Protocol::Valve(Engine::new(632_730)), GatheringSettings {
-        players: true,
+        players: GatherToggle::Enforce,
-        rules: false,
+        rules: GatherToggle::Skip,
         check_app_id: true,
     }.into_extra()),
     "battalion1944" => game!("Battalion 1944", 7780, Protocol::Valve(Engine::new(489_940))),
@@ -65,8 +65,8 @@ pub static GAMES: Map<&'static str, Game> = phf_map! {
     "codenamecure" => game!("Codename CURE", 27015, Protocol::Valve(Engine::new(355_180))),
     "colonysurvival" => game!("Colony Survival", 27004, Protocol::Valve(Engine::new(366_090))),
     "conanexiles" => game!("Conan Exiles", 27015, Protocol::Valve(Engine::new(440_900)), GatheringSettings {
-        players: false,
+        players: GatherToggle::Skip,
-        rules: true,
+        rules: GatherToggle::Enforce,
         check_app_id: true,
     }.into_extra()),
     "counterstrike" => game!("Counter-Strike", 27015, Protocol::Valve(Engine::new_gold_src(false))),
@@ -98,8 +98,8 @@ pub static GAMES: Map<&'static str, Game> = phf_map! {
     "quake2" => game!("Quake 2", 27910, Protocol::Quake(QuakeVersion::Two)),
     "q3a" => game!("Quake 3 Arena", 27960, Protocol::Quake(QuakeVersion::Three)),
     "risingworld" => game!("Rising World", 4254, Protocol::Valve(Engine::new(324_080)), GatheringSettings {
-        players: true,
+        players: GatherToggle::Enforce,
-        rules: false,
+        rules: GatherToggle::Skip,
         check_app_id: true,
     }.into_extra()),
     "ror2" => game!("Risk of Rain 2", 27016, Protocol::Valve(Engine::new(632_360))),
@@ -118,8 +118,8 @@ pub static GAMES: Map<&'static str, Game> = phf_map! {
     "unturned" => game!("Unturned", 27015, Protocol::Valve(Engine::new(304_930))),
     "unrealtournament" => game!("Unreal Tournament", 7778, Protocol::Gamespy(GameSpyVersion::One)),
     "valheim" => game!("Valheim", 2457, Protocol::Valve(Engine::new(892_970)), GatheringSettings {
-        players: true,
+        players: GatherToggle::Enforce,
-        rules: false,
+        rules: GatherToggle::Skip,
         check_app_id: true,
     }.into_extra()),
     "vrising" => game!("V Rising", 27016, Protocol::Valve(Engine::new(1_604_030))),

crates/lib/src/games/valve.rs

@@ -17,8 +17,8 @@ game_query_mod!(
     Engine::new(203_290),
     27020,
     GatheringSettings {
-        players: true,
+        players: GatherToggle::Enforce,
-        rules: false,
+        rules: GatherToggle::Skip,
         check_app_id: true,
     }
 );
@@ -53,8 +53,8 @@ game_query_mod!(
     Engine::new(440_900),
     27015,
     GatheringSettings {
-        players: false,
+        players: GatherToggle::Skip,
-        rules: true,
+        rules: GatherToggle::Enforce,
         check_app_id: true,
     }
 );
@@ -142,8 +142,8 @@ game_query_mod!(
     Engine::new(892_970),
     2457,
     GatheringSettings {
-        players: true,
+        players: GatherToggle::Enforce,
-        rules: false,
+        rules: GatherToggle::Skip,
         check_app_id: true,
     }
 );

crates/lib/src/protocols/types.rs

@@ -319,14 +319,16 @@ pub struct ExtraRequestSettings {
     ///
     /// Used by:
     /// - [valve::GatheringSettings#structfield.players]
+    /// - [unreal2::GatheringSettings#structfield.players]
     #[cfg_attr(feature = "clap", arg(long))]
-    pub gather_players: Option<bool>,
+    pub gather_players: Option<GatherToggle>,
     /// Whether to gather rule information.
     ///
     /// Used by:
     /// - [valve::GatheringSettings#structfield.rules]
+    /// - [unreal2::GatheringSettings#structfield.mutators_and_rules]
     #[cfg_attr(feature = "clap", arg(long))]
-    pub gather_rules: Option<bool>,
+    pub gather_rules: Option<GatherToggle>,
     /// Whether to check if the App ID is valid.
     ///
     /// Used by:
@@ -335,6 +337,31 @@ pub struct ExtraRequestSettings {
     pub check_app_id: Option<bool>,
 }
 
+/// Select how to go about gathering extra information via additional requests.
+///
+/// Used by:
+/// - [ExtraRequestSettings]
+/// - [valve::GatheringSettings]
+/// - [unreal2::GatheringSettings]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Default)]
+pub enum GatherToggle {
+    /// No request is sent for the relevant data. This option bypasses data
+    /// gathering.
+    #[default]
+    Skip,
+
+    /// A request will be sent, but errors are not treated as criticial.
+    /// In the case of an error, the operation will return a default value or
+    /// `None`.
+    Try,
+
+    /// A request will be sent, and any resulting errors will be propagated.
+    /// This option treats successful data gathering as mandatory.
+    Enforce,
+}
+
 impl ExtraRequestSettings {
     /// [Sets hostname](ExtraRequestSettings#structfield.hostname)
     pub fn set_hostname(mut self, hostname: String) -> Self {
@@ -348,12 +375,12 @@ impl ExtraRequestSettings {
         self
     }
     /// [Sets gather players](ExtraRequestSettings#structfield.gather_players)
-    pub const fn set_gather_players(mut self, gather_players: bool) -> Self {
+    pub const fn set_gather_players(mut self, gather_players: GatherToggle) -> Self {
         self.gather_players = Some(gather_players);
         self
     }
     /// [Sets gather rules](ExtraRequestSettings#structfield.gather_rules)
-    pub const fn set_gather_rules(mut self, gather_rules: bool) -> Self {
+    pub const fn set_gather_rules(mut self, gather_rules: GatherToggle) -> Self {
         self.gather_rules = Some(gather_rules);
         self
     }

crates/lib/src/protocols/unreal2/protocol.rs

@@ -2,7 +2,7 @@ use crate::buffer::{Buffer, StringDecoder};
 use crate::errors::GDErrorKind::PacketBad;
 use crate::protocols::types::TimeoutSettings;
 use crate::socket::{Socket, UdpSocket};
-use crate::utils::retry_on_timeout;
+use crate::utils::{maybe_gather, retry_on_timeout};
 use crate::GDResult;
 
 use super::{GatheringSettings, MutatorsAndRules, PacketKind, Players, Response, ServerInfo};
@@ -168,24 +168,22 @@ impl Unreal2Protocol {
         // Fetch the server info, this can only handle one response packet
         let mut server_info = self.query_server_info()?;
 
-        let mutators_and_rules = if gather_settings.mutators_and_rules {
+        let mutators_and_rules = maybe_gather!(
-            let response = self.query_mutators_and_rules()?;
+            gather_settings.mutators_and_rules,
+            self.query_mutators_and_rules()
+        )
+        .unwrap_or_default();
 
-            if let Some(password) = response.rules.get("GamePassword") {
+        if let Some(password) = mutators_and_rules.rules.get("GamePassword") {
-                let string = password.concat().to_lowercase();
+            let string = password.concat().to_lowercase();
-                server_info.password = string == "true";
+            server_info.password = string == "true";
-            }
+        }
 
-            response
+        let players = maybe_gather!(
-        } else {
+            gather_settings.players,
-            MutatorsAndRules::default()
+            self.query_players(Some(&server_info))
-        };
+        )
-
+        .unwrap_or_else(|| Players::with_capacity(0));
-        let players = if gather_settings.players {
-            self.query_players(Some(&server_info))?
-        } else {
-            Players::with_capacity(0)
-        };
 
         // TODO: Handle extra info parsing when we detect certain game types (or maybe
         // include that in gather settings).

crates/lib/src/protocols/unreal2/types.rs

@@ -1,6 +1,6 @@
 use crate::buffer::Buffer;
 use crate::errors::GDErrorKind::PacketBad;
-use crate::protocols::types::{CommonPlayer, CommonResponse, ExtraRequestSettings, GenericPlayer};
+use crate::protocols::types::{CommonPlayer, CommonResponse, ExtraRequestSettings, GatherToggle, GenericPlayer};
 use crate::protocols::GenericResponse;
 use crate::{GDError, GDResult};
 
@@ -209,16 +209,16 @@ impl CommonResponse for Response {
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct GatheringSettings {
-    pub players: bool,
+    pub players: GatherToggle,
-    pub mutators_and_rules: bool,
+    pub mutators_and_rules: GatherToggle,
 }
 
 impl GatheringSettings {
-    /// Default values are true for both the players and the rules.
+    /// Default values is attempt both players and rules.
     pub const fn default() -> Self {
         Self {
-            players: true,
+            players: GatherToggle::Try,
-            mutators_and_rules: true,
+            mutators_and_rules: GatherToggle::Enforce,
         }
     }
 

crates/lib/src/protocols/valve/mod.rs

@@ -28,7 +28,11 @@ macro_rules! game_query_mod {
     ($mod_name: ident, $pretty_name: expr, $engine: expr, $default_port: literal, $gathering_settings: expr) => {
         #[doc = $pretty_name]
         pub mod $mod_name {
-            use crate::protocols::valve::{Engine, GatheringSettings};
+            #[allow(unused_imports)]
+            use crate::protocols::{
+                types::GatherToggle,
+                valve::{Engine, GatheringSettings},
+            };
 
             crate::protocols::valve::game_query_fn!($pretty_name, $engine, $default_port, $gathering_settings);
         }

crates/lib/src/protocols/valve/protocol.rs

@@ -19,7 +19,7 @@ use crate::{
         },
     },
     socket::{Socket, UdpSocket},
-    utils::{retry_on_timeout, u8_lower_upper},
+    utils::{maybe_gather, retry_on_timeout, u8_lower_upper},
     GDErrorKind::{BadGame, Decompress, UnknownEnumCast},
     GDResult,
 };
@@ -470,13 +470,13 @@ fn get_response(
 
     Ok(Response {
         info,
-        players: match gather_settings.players {
+        players: maybe_gather!(
-            false => None,
+            gather_settings.players,
-            true => Some(client.get_server_players(&engine, protocol)?),
+            client.get_server_players(&engine, protocol)
-        },
+        ),
-        rules: match gather_settings.rules {
+        rules: maybe_gather!(
-            false => None,
+            gather_settings.rules,
-            true => Some(client.get_server_rules(&engine, protocol)?),
+            client.get_server_rules(&engine, protocol)
-        },
+        ),
     })
 }

crates/lib/src/protocols/valve/types.rs

@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 
-use crate::protocols::types::{CommonPlayer, CommonResponse, ExtraRequestSettings, GenericPlayer};
+use crate::protocols::types::{CommonPlayer, CommonResponse, ExtraRequestSettings, GatherToggle, GenericPlayer};
 use crate::GDErrorKind::UnknownEnumCast;
 use crate::GDResult;
 use crate::{buffer::Buffer, protocols::GenericResponse};
@@ -283,17 +283,18 @@ impl Engine {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct GatheringSettings {
-    pub players: bool,
+    pub players: GatherToggle,
-    pub rules: bool,
+    pub rules: GatherToggle,
     pub check_app_id: bool,
 }
 
 impl GatheringSettings {
-    /// Default values are true for both the players and the rules.
+    /// Default values are try to gather but don't fail on timeout for both
+    /// players and rules.
     pub const fn default() -> Self {
         Self {
-            players: true,
+            players: GatherToggle::Try,
-            rules: true,
+            rules: GatherToggle::Try,
             check_app_id: true,
         }
     }

crates/lib/src/utils.rs

@@ -29,11 +29,62 @@ pub fn retry_on_timeout<T>(mut retry_count: usize, mut fetch: impl FnMut() -> GD
     Err(last_err)
 }
 
+/// Run gather_fn based on the value of gather_toggle.
+///
+/// # Parameters
+/// - `gather_toggle` should be an expression resolving to a
+///   [crate::protocols::types::GatherToggle].
+/// - `gather_fn` should be an expression that returns a [crate::GDResult].
+///
+/// # States
+/// - [DontGather](crate::protocols::types::GatherToggle::DontGather) - Don't
+///   run gather function, returns None.
+/// - [AttemptGather](crate::protocols::types::GatherToggle::AttemptGather) -
+///   Runs the gather function, if it returns an error return None, else return
+///   Some.
+/// - [Required](crate::protocols::types::GatherToggle::Required) - Runs the
+///   gather function, if it returns an error propagate it using the `?`
+///   operator, else return Some.
+///
+/// # Examples
+///
+/// ```ignore,Doctests cannot access private items
+/// use gamedig::protocols::types::GatherToggle;
+/// use gamedig::utils::maybe_gather;
+///
+/// let query_fn = || { Err("Query error") };
+///
+/// // query_fn() is not called
+/// let response = maybe_gather!(GatherToggle::DontGather, query_fn());
+/// assert!(response.is_none());
+///
+/// // query_fn() is called but Err is converted to None
+/// let response = maybe_gather!(GatherToggle::AttemptGather, query_fn());
+/// assert!(response.is_none());
+///
+/// // query_fn() is called and Err is propagated.
+/// let response = maybe_gather!(GatherToggle::Required, query_fn());
+/// unreachable!();
+/// ```
+macro_rules! maybe_gather {
+    ($gather_toggle: expr, $gather_fn: expr) => {
+        match $gather_toggle {
+            crate::protocols::types::GatherToggle::Skip => None,
+            crate::protocols::types::GatherToggle::Try => $gather_fn.ok(),
+            crate::protocols::types::GatherToggle::Enforce => Some($gather_fn?),
+        }
+    };
+}
+
+pub(crate) use maybe_gather;
+
 #[cfg(test)]
 mod tests {
     use super::retry_on_timeout;
     use crate::{
-        GDErrorKind::{PacketBad, PacketReceive, PacketSend},
+        protocols::types::GatherToggle,
+        GDError,
+        GDErrorKind::{self, PacketBad, PacketReceive, PacketSend},
         GDResult,
     };
 
@@ -105,4 +156,53 @@ mod tests {
         assert!(r.is_err());
         assert_eq!(r.unwrap_err().kind, PacketBad);
     }
+
+    fn gather_success(n: i32) -> GDResult<i32> { Ok(n) }
+
+    fn gather_fail(err: &'static str) -> GDResult<i32> { Err(GDErrorKind::PacketSend.context(err)) }
+
+    #[test]
+    fn gather_success_dont_gather() -> GDResult<()> {
+        let result = maybe_gather!(GatherToggle::Skip, gather_success(5));
+        assert!(result.is_none());
+        Ok(())
+    }
+
+    #[test]
+    fn gather_success_attempt_gather() -> GDResult<()> {
+        let result = maybe_gather!(GatherToggle::Try, gather_success(10));
+        assert_eq!(result, Some(10));
+        Ok(())
+    }
+
+    #[test]
+    fn gather_success_required() -> GDResult<()> {
+        let result = maybe_gather!(GatherToggle::Enforce, gather_success(15));
+        assert_eq!(result, Some(15));
+        Ok(())
+    }
+
+    #[test]
+    fn gather_fail_dont_gather() -> GDResult<()> {
+        let result = maybe_gather!(GatherToggle::Skip, gather_fail("dont"));
+        assert!(result.is_none());
+        Ok(())
+    }
+
+    #[test]
+    fn gather_fail_attempt_gather() -> GDResult<()> {
+        let result = maybe_gather!(GatherToggle::Try, gather_fail("attempt"));
+        assert!(result.is_none());
+        Ok(())
+    }
+
+    #[test]
+    fn gather_fail_required() {
+        let inner = || {
+            let result = maybe_gather!(GatherToggle::Enforce, gather_fail("required"));
+            assert_eq!(result, Some(10));
+            Ok::<(), GDError>(())
+        };
+        assert!(inner().is_err());
+    }
 }