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

Input payload differences between Rest and RT clients #23

Closed leoncarl closed 4 years ago

leoncarl commented 4 years ago

We've noted that: The Rest client:

void RestClient::rpc( NSessionPtr session, const std::string & id, const opt::optional<std::string>& payload, std::function<void(const NRpc&)> successCallback, ErrorCallback errorCallback)

works with a regular JSON encoded string for payload where the RT Client:

void NRtClient::rpc(const std::string & id, const opt::optional<std::string>& payload, std::function<void(const NRpc&)> successCallback, RtErrorCallback errorCallback)

Will not work unless you JSON encode the JSON encoded string (as second time).

This has caused some head scratching a few times since the Nakama server will return an error (which does not end up in the error log) if you provide a JSON encoded string (only encoded once).

It would be really nice if those two rpc functions acted the same (they have the same signature) and the RT one did the extra encoding/escaping internally as needed.

Thoughts?

zyro commented 4 years ago

Hi @leoncarl! Can you double-check if the two aren't mixed up by any chance? I'd expect the REST option to need double-encoding, and the realtime path to work when encoding only once - this is the opposite of what you're reporting.

The reason the REST side is awkward is the GRPC Gateway layer expects that string to decode to a JSON string, hence the awkward double encoding. This has been improved in Nakama 2.7.0 but the original behaviour is still the default. We'll look into switching the default to single-encoded in a future client update, hopefully without breaking expected client behaviour.

leoncarl commented 4 years ago

Apologies, yes, I mixed them up when writing it up. Just let us know if the default behavior changes down the road and we will simplify on our end. Cheers.

Dimon4eg commented 4 years ago

Fixed in 2.2.3