heroiclabs / nakama-unreal

Unreal client for Nakama server.
Apache License 2.0
200 stars 61 forks source link

Use metadata field when sending JoinMatch metadata (bugfix) #115

Closed intinig closed 9 months ago

intinig commented 9 months ago

This might be a weird PR because it's necessary bugfix in order to make JoinMatch work for us.

Summary

We recently did a full upgrade of Nakama and supporting libs in order to stop using a Nakama fork and get rid of the old nakama-core-based code and move to the new code that uses native Unreal types. Unfortunately our game stopped working and every time we entered a match queue and found a match we were disconnected.

A little investigation brought us to this point in nakama server code, specifically to the Received malformed payload error:

request := &rtapi.Envelope{}
switch s.format {
    case SessionFormatProtobuf:
        err = proto.Unmarshal(data, request)
    case SessionFormatJson:
        fallthrough
    default:
        err = s.protojsonUnmarshaler.Unmarshal(data, request)
}
if err != nil {
    // If the payload is malformed the client is incompatible or misbehaving, either way disconnect it now.
    s.logger.Warn("Received malformed payload", zap.Binary("data", data))
    reason = "received malformed payload"
    break
}

Attaching the debugger to a test Nakama instance to inspect the error gave us some insight into the issue, the party_id key was unexpected.

This is an example payload we were sending with JoinMatch:

{
    "match_join":
    {
        "match_id": "85781210-ecbc-4cd6-90e8-6dd44d9041b6.nakama1",
        "party_id": "414b71ab-9524-4bd0-87b6-ca408bfe90b5.nakama1"        
        },
    "cid": "7"
}

A quick check of the proto in nakama-common confirmed our suspicion that JoinMatch wasn't playing nice:

message MatchJoin {
  oneof id {
    // The match unique ID.
    string match_id = 1;
    // A matchmaking result token.
    string token = 2;
  }
  // An optional set of key-value metadata pairs to be passed to the match handler, if any.
  map<string, string> metadata = 3;
}

Here we are expecting the metadata passed to MatchJoin as a metadata field, not as extra fields added to the main message. We tested the code change in this PR and it fixes our issue.

The reason why I say this PR might be weird is that I searched among known issues but it seems like we were the only ones with this issue, so I still think it might be us doing something weird?

CLAassistant commented 9 months ago

CLA assistant check
All committers have signed the CLA.

lugehorsam commented 9 months ago

Thanks @intinig -- this was indeed a bug in the new client.

intinig commented 9 months ago

Glad I could help :)