Member codec missing version check for field address_map [API-1078]

[ihsandemir]

address_map field was added after protocol version 2.0 and hence the custom codec should check against this. See Java codec.

This is not a problem when working with latest patch server version 4.0.3 since that server encodes the field.

[yemreinci]

Is there a reason for not auto-generating custom codecs as other clients do? Errors like these can be avoided if codecs are auto-generated.

[ihsandemir]

It was less code this way but yes, we can change it to be auto generated.

[yemreinci]

It was less code this way but yes, we can change it to be auto generated.

How is manually writing codecs less code? If you use generation you'll write the template once and generate for all types.

[ihsandemir]

It was less code this way but yes, we can change it to be auto generated.

How is manually writing codecs less code? If you use generation you'll write the template once and generate for all types.

Hand written code vs generated code, not the template code. But yes, as long as we do it efficiently with least possible code, I am OK with generating. Generated custom codecs had their associated classes but they actually boiled down to methods only on ClientMessage in hand written code, and the generated non-custom codecs only had msg.set/msg.get calls in the generated codecs, which looked like a simpler code.

As I said, I am not strong on continuing on hand written codecs and we can change it especially to prevent backward compatibility issues.

[yemreinci]

Hand written code vs generated code, not the template code. But yes, as long as we do it efficiently with least possible code, I am OK with generating.

Does it matter how long the generated code is? Longer code doesn't necessarily mean it will run slower.

Generated custom codecs had their associated classes but they actually boiled down to methods only on ClientMessage in hand written code, and the generated non-custom codecs only had msg.set/msg.get calls in the generated codecs, which looked like a simpler code.

Codecs don't have to be classes, C++ has free functions. We can have something like this for example:

member_info_codec.h:

namespace protocol { namespace codecs {

/// decodes and returns a member_info from a ClientMessage
template<>
member_info decode<member_info>(ClientMessage& msg);

}}

This is not much different than implementing it as a method of ClientMessage performance-wise, but will be cleaner and easier to generate as we can separate things into multiple header files and include only the necessary ones when we need them. I think ClientMessage.h is very hard to read as all the protocol encoding/decoding logic is within the class definition.

And finally, yes, codec implementations are very simple (just some get/set calls) but generating it would be even simpler and error-free. I think we should reconsider the codec generation design for C++ and change the design so that we generate all the things we can generate (including custom codecs and decoders). We can do benchmarks to ensure generated code doesn't run slower. I think we can do this after we release 5.0 as the changes wouldn't effect the public interface.

[github-actions[bot]]

Internal Jira issue: API-1078