heroiclabs / nakama-cpp

Generic C/C++ client for Nakama server.
https://heroiclabs.com/docs/cpp-client-guide
Apache License 2.0
69 stars 25 forks source link

Error cancelling matchmaking from the same session ID using Nakama CPP client #49

Closed rpvela closed 2 years ago

rpvela commented 2 years ago

Description

Using nakama C wrapper in nakama's CPP client produces an error when trying to cancel matchmaking using the same session. Debugger output when running local nakama suggests that the matchmaking handler expects the Party ID or Session ID to be present, exclusively -- not both at the same time.

What's the expected flow here? Are we using wrong session to initiate/cancel the matchmaking?

Steps to Reproduce

Using C NakamaWrapper

RTClient->addMatchmakerParty(
        TCHAR_TO_UTF8(*PartyID), TCHAR_TO_UTF8(*matchQuery), minmatchedCount, maxMatchedCount, stringProperties, {}, SuccessCallback, ErrorCallback);
...
RTClient->removeMatchmakerParty(TCHAR_TO_UTF8(*TicketId), SuccessCallback, ErrorCallback);

This is the same client using the weak pointer to the NRtClientInterface

{"level":"debug","ts":"2022-01-11T17:08:45.001Z","caller":"server/pipeline.go:65","msg":"Received *rtapi.Envelope_PartyMatchmakerAdd message","uid":"cded365b-b40c-455a-bbfb-faf7f8aed966","sid":"1c95065c-7301-11ec-9dac-7106fdcb5b46","cid":"1","message":{"PartyMatchmakerAdd":{"party_id":"8212a156-5e77-4c1a-b93a-c6c10f6dd5c1.nakama1","min_count":2,"max_count":4,"query":"properties.serverVersion:057a3fe6336667f7f4ebe6d532eec1ed183cbbd0","string_properties":{"serverVersion":"057a3fe6336667f7f4ebe6d532eec1ed183cbbd0"}}}}
{"level":"debug","ts":"2022-01-11T17:08:45.002Z","caller":"server/session_ws.go:395","msg":"Sending *rtapi.Envelope_PartyMatchmakerTicket message","uid":"cded365b-b40c-455a-bbfb-faf7f8aed966","sid":"1c95065c-7301-11ec-9dac-7106fdcb5b46","envelope":"cid:\"1\" party_matchmaker_ticket:{party_id:\"8212a156-5e77-4c1a-b93a-c6c10f6dd5c1.nakama1\" ticket:\"61a33496-2503-4446-a619-8e8584814c13\"}"}
{"level":"debug","ts":"2022-01-11T17:09:05.987Z","caller":"server/pipeline.go:65","msg":"Received *rtapi.Envelope_MatchmakerRemove message","uid":"cded365b-b40c-455a-bbfb-faf7f8aed966","sid":"1c95065c-7301-11ec-9dac-7106fdcb5b46","cid":"2","message":{"MatchmakerRemove":{"ticket":"61a33496-2503-4446-a619-8e8584814c13"}}}
{"level":"debug","ts":"2022-01-11T17:09:05.987Z","caller":"server/session_ws.go:393","msg":"Sending error message","uid":"cded365b-b40c-455a-bbfb-faf7f8aed966","sid":"1c95065c-7301-11ec-9dac-7106fdcb5b46","payload":"CgEyWh8IAxIbTWF0Y2htYWtlciB0aWNrZXQgbm90IGZvdW5k"}

Debugger: image

The function that errors is here https://github.com/heroiclabs/nakama/blob/df2e6bfc1652671ebea2644cff165a22c312fb1d/server/matchmaker.go#L541

func (m *LocalMatchmaker) RemoveSession(sessionID, ticket string) error {
    m.Lock()

    index, ok := m.indexes[ticket]
    if !ok || index.PartyId != "" || index.SessionID != sessionID {
        // Ticket did not exist, or the caller was not the ticket owner - for example a user attempting to remove a party ticket.
        m.Unlock()
        return runtime.ErrMatchmakerTicketNotFound

Overriding the session ID makes the match cancel properly https://github.com/rpvela/nakama/commit/3ec6e0cba87ae9f36456512bdbbcee3081764a57

Expected Result

StopPartyMatchmaking::<lambda_3661080582e47bd6dee52ce2181826bf>::operator (): Cancelled matchmaking.

Actual Result

StopPartyMatchmaking::<lambda_7bc4c986aef746a6d3dcd754700333c0>::operator (): Error cancelling matchmaking 'Matchmaker ticket not found'

Context

Your Environment

redbaron commented 2 years ago

RTClient->removeMatchmakerParty calls wrong API method. Currently it is exact copy of RTClient->removeMatchmaker and doesn't accept PartyID as an argument.

@rpvela, would you be able to test a fix from the nakama-cpp branch before we cut new release?

redbaron commented 2 years ago

Fix is in https://github.com/heroiclabs/nakama-cpp/tree/party-mm-remove-fix branch

rpvela commented 2 years ago

Thanks! I will take a look at it, and makes sense

rpvela commented 2 years ago

Can confirm this fixed the issue on our end! @redbaron