gamedig / rust-gamedig

Game Server Query Library.
https://crates.io/crates/gamedig
MIT License
38 stars 11 forks source link

BF1942 fails to parse players #88

Open CosminPerRam opened 1 year ago

CosminPerRam commented 1 year ago

Describe the bug BF1942 fails in extract_players, backtrace: { fn: "gamedig::protocols::gamespy::protocols::one::protocol::extract_players", file: ".\src\protocols\gamespy\protocols\one\protocol.rs", line: 132 }

Steps To Reproduce Use the master_querant example to query this server: 51.81.48.224. Command: cargo r --example master_querant bf1942 51.81.48.224

Expected behavior Clean response output.

Additional context Printing player_data (the hashmap where it gets its values), it seems that it does not contain the playername key (also note that node also uses name and player for this, neither these are present, so this might be a parsing issue?).

Douile commented 1 year ago

Wasn't able to replicate this, instead I get (for the same server):

``` { fn: "gamedig::protocols::gamespy::protocols::one::protocol::get_server_values", file: "./src/protocols/gamespy/protocols/one/protocol.rs", line: 76 }, ``` ``` GDError{ kind=PacketBad backtrace=Backtrace [ { fn: ">::from", file: "./src/errors.rs", line: 93 }, { fn: ">::into", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/convert/mod.rs", line: 717 }, { fn: "gamedig::protocols::gamespy::protocols::one::protocol::get_server_values", file: "./src/protocols/gamespy/protocols/one/protocol.rs", line: 76 }, { fn: "gamedig::protocols::gamespy::protocols::one::protocol::query_vars", file: "./src/protocols/gamespy/protocols/one/protocol.rs", line: 195 }, { fn: "gamedig::protocols::gamespy::protocols::one::protocol::query", file: "./src/protocols/gamespy/protocols/one/protocol.rs", line: 202 }, { fn: "gamedig::games::query_with_timeout", file: "./src/games/mod.rs", line: 175 }, { fn: "generic::generic_query", file: "./examples/generic.rs", line: 23 }, { fn: "generic::main", file: "./examples/generic.rs", line: 46 }, { fn: "core::ops::function::FnOnce::call_once", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/ops/function.rs", line: 250 }, { fn: "std::sys_common::backtrace::__rust_begin_short_backtrace", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/sys_common/backtrace.rs", line: 135 }, { fn: "std::rt::lang_start::{{closure}}", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/rt.rs", line: 166 }, { fn: "core::ops::function::impls:: for &F>::call_once", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/ops/function.rs", line: 284 }, { fn: "std::panicking::try::do_call", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs", line: 500 }, { fn: "std::panicking::try", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs", line: 464 }, { fn: "std::panic::catch_unwind", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panic.rs", line: 142 }, { fn: "std::rt::lang_start_internal::{{closure}}", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/rt.rs", line: 148 }, { fn: "std::panicking::try::do_call", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs", line: 500 }, { fn: "std::panicking::try", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs", line: 464 }, { fn: "std::panic::catch_unwind", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panic.rs", line: 142 }, { fn: "std::rt::lang_start_internal", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/rt.rs", line: 148 }, { fn: "std::rt::lang_start", file: "/rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/rt.rs", line: 165 }, { fn: "main" }, { fn: "__libc_start_main" }, { fn: "_start" }, ] } ```
CosminPerRam commented 1 year ago

I think #72 might be related (and/or relevant) here.

Douile commented 1 year ago

Ah, it seems after querying for a while I was able to replicate the issue (I think maybe the players in the servers changed and the issue I was having went away).

I found the problem with this one: The early return match statement doesn't include "playername" which is checked for later down, so the "playername" properties never get stored.

Logging the early return shows quite a few things get ignored like:

However after fixing the playername issue I immediately run into another where player_data["face"] is expected but not found. So I will keep looking and make it into a larger patch.

CosminPerRam commented 1 year ago

Yeah, the current GS players fields implementation doesnt really match up with the node version, as you said, the statement doesnt include playername (and other match statements are in the same case (score/teamname).

Regarding the 'extra' fields, such as keyhash or possibly face/skin/ngsecret (that could be particular to one game) we could change it up to have a hashmap with these and their values rather than Option<String> those.