Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BF1942 fails to parse players #88

Open
CosminPerRam opened this issue Aug 15, 2023 · 4 comments
Open

BF1942 fails to parse players #88

CosminPerRam opened this issue Aug 15, 2023 · 4 comments
Labels
bug Something isn't working game This is something regarding a game protocol This is something regarding a protocol v0.6.X Things about v0.6.X
Milestone

Comments

@CosminPerRam
Copy link
Member

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?).

@CosminPerRam CosminPerRam added bug Something isn't working protocol This is something regarding a protocol game This is something regarding a game labels Aug 15, 2023
@Douile
Copy link
Collaborator

Douile commented Aug 17, 2023

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: "<gamedig::errors::GDError as core::convert::From<gamedig::errors::GDErrorKind>>::from", file: "./src/errors.rs", line: 93 },
      { fn: "<T as core::convert::Into<U>>::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::<impl core::ops::function::FnOnce<A> 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
Copy link
Member Author

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

@Douile
Copy link
Collaborator

Douile commented Aug 17, 2023

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:

  • score
  • keyhash
  • kills
  • teamname

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
Copy link
Member Author

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.

Douile added a commit to Douile/rust-gamedig that referenced this issue Aug 21, 2023
Douile added a commit to Douile/rust-gamedig that referenced this issue Aug 21, 2023
These fields seem to be missing from bf1942 queries so make them
optional.
Douile added a commit to Douile/rust-gamedig that referenced this issue Sep 27, 2023
Douile added a commit to Douile/rust-gamedig that referenced this issue Sep 27, 2023
These fields seem to be missing from bf1942 queries so make them
optional.
Douile added a commit to Douile/rust-gamedig that referenced this issue Sep 27, 2023
Douile added a commit to Douile/rust-gamedig that referenced this issue Sep 27, 2023
These fields seem to be missing from bf1942 queries so make them
optional.
CosminPerRam pushed a commit that referenced this issue Sep 27, 2023
* [Protocol] Gamespy1 don't skip "playername" field (#88)

* [Protocol] Gamespy1 make more player fields optional (#88)

These fields seem to be missing from bf1942 queries so make them
optional.
@cainthebest cainthebest added this to the Backlog milestone Sep 30, 2024
@cainthebest cainthebest added the v0.6.X Things about v0.6.X label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working game This is something regarding a game protocol This is something regarding a protocol v0.6.X Things about v0.6.X
Projects
None yet
Development

No branches or pull requests

3 participants