force_full_app_update of CMsgClientPICSChangesSinceResponse is not propagated

[t9t]

In PICSChangesCallback, the property RequiresFullUpdate is set to the value of force_full_update. However, CMsgClientPICSChangesSinceResponse also has force_full_app_update and force_full_package_update. These are not available in the code at all (and I noticed that when force_full_app_update is true, the force_full_update remains false).

Location of force_full_app_update:

SteamKit/Resources/Protobufs/steamclient/steammessages_clientserver.proto

Line 971 in 7640dfe

optional bool force_full_app_update = 6;

Location where RequiresFullUpdate is set:

SteamKit/SteamKit2/SteamKit2/Steam/Handlers/SteamApps/Callbacks.cs

Line 394 in 5fbf660

RequiresFullUpdate = msg.force_full_update;

[yaakov-h]

Just to make sure I understand this: the issue here is that force_full_app_update and force_full_package_update aren't exposed on the callback?

[JustArchi]

If I understood it right t9t wants to change:

RequiresFullUpdate = msg.force_full_update;

Into:

RequiresFullUpdate = msg.force_full_update || msg.force_full_app_update || msg.force_full_package_update;

And if you ask me it makes sense, since as noticed above - force_full_app_update might be true while force_full_update is still false. If we need to distinguish between those 3 then we should add missing 2 into the response, but I believe just fixing RequiresFullUpdate would be enough.

[t9t]

@yaakov-h yes, that's exactly correct.

@JustArchi I would like it more if all the properties were exposed in the callback, so that the user can decide for themselves which properties they would like to use or not.

Changing the way that the value of RequiresFullUpdate is determined is a breaking API change. Most of the time force_full_update is just false even with a severely outdated change number, and even when both force_full_app_update or force_full_package_update are true (maybe it's an older, now unused property?) . I'm not sure how comfortable you are with that.

[JustArchi]

Most of the time force_full_update is just false even with a severely outdated change number, and even when both force_full_app_update or force_full_package_update are true (maybe it's an older, now unused property?)

I assumed as much and this is the reason why I'd rewrite that property to be actually useful and not just outdated placeholder. Still, from library point of view it'd be probably better to expose all 3, so I'm with you on that, although I also like how SK2 can add logic and make those generic Steam responses actually useful for the developer, so he has a flag that tells him if app requires full update or not, not 3 generic flags and "guess yourself what is the right one" behaviour. If somebody needs that generic behaviour then he can send the request and parse the response himself, I've always seen SK2 functions as a helper layer between raw requests with responses, and the developer and his app itself. In this case it'd be better if the callback produced useful pre-parsed properties and not just raw values - we can already ask for raw values by sending the request.

[t9t]

I haven't dug into the code much, and this is slightly off-topic, but I wonder if it would be possible to include the raw message objects in the callbacks, so that it would be available in the event that someone might want to use some arcane properties.

[yaakov-h]

You can always implement a ClientMsgHandler and get at the raw message yourself.

My current thinking is just to add two new properties to the callback object.

[xPaw]

This was fixed in #865