Use non-legacy service methods (and some refactoring of messages/packets handling) #1036
Conversation
SteamKit2/SteamKit2/Steam/Handlers/SteamUnifiedMessages/Callbacks.cs
Outdated
Show resolved
Hide resolved
SteamKit2/SteamKit2/Steam/Handlers/SteamUnifiedMessages/Callbacks.cs
Outdated
Show resolved
Hide resolved
| @@ -50,13 +50,13 @@ public string RpcName | |||
| public string MethodName { get; private set; } | |||
|
|
|||
|
|
|||
| internal ServiceMethodResponse( JobID jobID, EResult result, CMsgClientServiceMethodLegacyResponse resp ) | |||
| internal ServiceMethodResponse( JobID jobID, EResult result, ClientMsgProtobuf response, IPacketMsg packetMsg ) | |||
There was a problem hiding this comment.
Can we pass in just the target job name rather than the whole proto message object, considering that all we do with it is immediately dot into the target job name?
Alternatively, why deserialise it to get to the target job name, when PacketClientMsgProtobuf (IPacketMsg subclass) has already deserialised the proto header? We shouldn't need to deserialise it twice.
There was a problem hiding this comment.
So it does. Then why does ClientMsgProtobuf also deserialize the header?
| { | ||
| var response = new ClientMsgProtobuf<CMsgClientServiceMethodLegacyResponse>( packetMsg ); | ||
| var callback = new ServiceMethodResponse(response.TargetJobID, (EResult)response.ProtoHeader.eresult, response.Body); | ||
| var response = new ClientMsgProtobuf( packetMsg ); |
There was a problem hiding this comment.
See above re. deserialising the message twice - the IPacketMsg object here should already contain what we need.
There was a problem hiding this comment.
It seems to contain the raw payload, which would mean I'd still have to skip over the header when deserializing the body.
xPaw
changed the title
|
FYI you mentioned #266, but that issue talks about GC messages which this PR doesn't touch. |
|
Right, I meant the same concept (since all those message types are very similar) - being able to construct a message by giving it a body, rather than having the message allocate the body and then having to manipulate its properties through the Body getter. |
Ref: #909
This appears to work, but the implementation is questionable at best. I haven't tested ServiceMethodSendToClient.
GetDeserializedResponsecould be removed if responses were handled similarly to how they're done for notifications here: https://github.com/xPaw/SteamKit/blob/c3964e89564575083dfd30c354394ec13131301c/SteamKit2/SteamKit2/Steam/Handlers/SteamUnifiedMessages/SteamUnifiedMessages.cs#L182However, it will not work if consumers have their own protobufs that SK doesn't have.