Update clarity-protobuf

[tsproskurnin]

In the latest patch, the protobuf was updated, and the hero_id was changed from uint32 to int32, which caused the hero_id in the parser to become incorrect.

[spheenik]

According to the protobuf documentation under https://protobuf.dev/programming-guides/encoding/ there should be no difference in encoding here:

As you saw in the previous section, all the protocol buffer types associated with wire type 0 are encoded as varints. However, varints are unsigned, so the different signed types, sint32 and sint64 vs int32 or int64, encode negative integers differently.

The intN types encode negative numbers as two’s complement, which means that, as unsigned, 64-bit integers, they have their highest bit set.

Are you getting negative numbers since that patch?

[STRATZ-Ken]

Confirmed. All replays are currently broken.

[tsproskurnin]

@spheenik All IDs are multiplied by 2, I assume this is related to storing the number in binary code.

11 -> 110 (3 -> 6)

[spheenik]

Hmm. I don't know how to change this. As per protobuf spec, unsigned and signed integers are encoded in exactly the same way. Which means even if I change the protobufs, you will still get the wrong value.

@tsproskurnin Have you checked that the binary encoding has changed? Protobuf encodes at least 8 bits for each signed or unsigned int, where 7 bits are payload. Both 3 and 6 can easily be encoded in 7 bits.

Valve always do stuff like this.

[tsproskurnin]

No, have not checked binary encoding.

So, you are right about encoding signed and unsigned numbers. Then i have no idea why it encode wrong in clarity.

[azhezzzz]

But I found that the hero_Id of picks_bans in Demo.CDemoFileInfo info = Clarity.infoForFile(args[0]) is correct. I'm not sure if this helps

[spheenik]

The thing is there could be two things leading to this:

1. clarity decodes wrong value
2. CS client encodes wrong value, which is decoded by clarity correctly

I suspect it to be 2. Maybe the lower bit (which causes multiplication by 2) encodes something new? Do you see the lowest bit to be 1 in any case?

[spheenik]

But I found that the hero_Id of picks_bans in Demo.CDemoFileInfo info = Clarity.infoForFile(args[0]) is correct. I'm not sure if this helps

But isn't this the only place where the CHeroSelectEvent is used? @tsproskurnin: Can you confirm that you also use infoForFile()?

[tsproskurnin]

No, im not using infoForFile()

[spheenik]

Then how do you get at a CHeroSelectEvent? Is it an embedded message?

[Dmytro4]

I can confirm the same issue that seems visible even in Clarity Analyzer (I can not watch new replays properly since it drops some errors but I can view class data).

[spheenik]

Well, so we are talking about m_nSelectedHeroID in CDOTA_PlayerResource? Because this seems to have been encoded as a int32, and Valve seem to have changed this to be a HeroID_t. That way, I could just implement a different encoder to fix this. @tsproskurnin Can you confirm this is what you also mean?

[tsproskurnin]

Yes, thats it.

Also it may be the same problem with m_pGameRules.m_BannedHeroes and m_pGameRules.m_SelectedHeroes in CDOTAGamerulesProxy

[STRATZ-Ken]

Any quick fix I can make to get parsing running for STRATZ?

[ZainJavedDev]

@howardchung Hey, how is odota still parsing when clarity is not.

[Dmytro4]

Any quick fix I can make to get parsing running for STRATZ?

you can get real hero names by getting entity of "SelectedHero" and checking its class name so you can map it with your own list of heroname=>id

[howardchung]

I have not investigated yet, seeing elevated errors but not 100% failure. It's possible we're only using the hero ID in a non-critical way

[spheenik]

I'm unsure about the fix. Doesn't this break old replays? I will have to test first, but I do not have time at the moment. As a workaround, you could check if the type is HeroID_t and divide by 2?

[STRATZ-Ken]

I have not tested an old replay, but it works for new replays.

[spheenik]

So, I finally came aroung to testing this:

I created two messages:

message TestInt {
  optional int32 value = 1;
}

message TestUInt {
  optional uint32 value = 1;
}

and created the following code to test this:

import skadistats.clarity.wire.csgo.s2.proto.CSGOS2ClarityMessages;
import java.util.Arrays;
public class Test {
    public static void main(String[] args) {
        var intMsg = CSGOS2ClarityMessages.TestInt.newBuilder()
                .setValue(987654321)
                .build()
                .toByteArray();

        var uintMsg = CSGOS2ClarityMessages.TestUInt.newBuilder()
                .setValue(987654321)
                .build()
                .toByteArray();

        System.out.println(Arrays.equals(intMsg, uintMsg));
    }
}

The output is true, which means the problem is that Valve decided to change the semantics of the field, which is something I will not address in clarity.

[spheenik]

fixed in 3.1.0