kebabtent / pogoprotos-php

Compiled PHP protobufs for pokemon go
MIT License
22 stars 8 forks source link

Reading GameMaster file fails #17

Open iBotPeaches opened 8 years ago

iBotPeaches commented 8 years ago

So I started with simply reading in a raw response dump like this.

$raw = fopen(public_path('item_templates/raw_rpc'), 'r');

$message = new ResponseEnvelope($raw);
$gamemaster = new DownloadItemTemplatesResponse($message->getReturnsList()[0]);

This fails with

Expected wire type 0 but got 2 for type 5.

With full stack of

in WireFormat.php line 77
at WireFormat::assertWireType('2', '5') in BadgeSettings.php line 325
at BadgeSettings->readFrom(object(ReadContext)) in ItemTemplate.php line 1062
at ItemTemplate->readFrom(object(ReadContext)) in DownloadItemTemplatesResponse.php line 333
at DownloadItemTemplatesResponse->readFrom(object(ReadContext)) in AbstractMessage.php line 29
at AbstractMessage->__construct(object(Stream)) in Controller.php line 25
at Controller->postIndex(object(Request))

I began investigating this, stepping through the file as it fails here - https://github.com/jaspervdm/pogoprotos-php/blob/master/src/POGOProtos/Settings/Master/BadgeSettings.php#L325

This corresponds to - https://github.com/AeonLucid/POGOProtos/blob/master/src/POGOProtos/Settings/Master/BadgeSettings.proto

message BadgeSettings {
    .POGOProtos.Enums.BadgeType badge_type = 1;
    int32 badge_rank = 2;
    repeated int32 targets = 3;
}

Is the datatype wrong or how should I be properly handling groups? The beginning of the file I'm reading is as follows.

➜  pokemongo-dumps git:(master) ✗ protoc --decode_raw < raw_rpc | more
1: 1
2: xxxx
6 {
  1: 6
  2 {
    1: 1
  }
}
100 { [DownloadItemTemplatesResponse]
  1: 1 [sucess]
  2 { [ItemTemplate]
    1: "BADGE_BATTLE_ATTACK_WON" [template_id]
    10 { [BadgeSettings]
      1: 13 [BadgeType]
      2: 4 [BadgeRank]
      3: "\nd\350\007" [Targets (repeated)]
    }
  }
....

So it reads success, template_id, badge_type and badge_rank, then fails on number 3 (targets).

DrDelay commented 8 years ago

Can you post the whole _rawrpc file for some testing on this matter?

iBotPeaches commented 8 years ago

It's in a zip so I could upload it. Unzip for file.

raw_rpc.zip

jaspervdm commented 8 years ago

I just did a DownloadItemTemplates request myself and get the same error. Will investigate further.

Ni42 commented 8 years ago

I suddenly have this problem inside a GetMapObjectsResponse, too. It fails on POGOProtos\Map\Fort\FortData.php(1054), expected wire type 0 but got 2 for type 14 (that's field index 12)

Corresponding raw proto (excerpt):

    3 {
      1: "<fort id>"
      2: 1471701450286
      3: <latitude>
      4: <longitude>
      8: 1
      9: 1
      12: "\365\003"
    }

This field is now listed as a repeated field in the proto: https://github.com/AeonLucid/POGOProtos/blob/master/src/POGOProtos/Map/Fort/FortData.proto

Is it possible this has something to do with it, since wire type 2 is also "repeated packed" data. https://github.com/google/protobuf/releases/tag/v3.0.0

Changed repeated primitive fields to use packed serialization by default in proto3 (implemented for C++, Java, Python in this release). The user can still disable packed serialization by setting packed to false for now.

DrDelay commented 8 years ago

Seems to have to do with the packed thingy @Ni42 mentioned. I edited the targets in BadgeSettings.proto like this:

repeated int32 targets = 3 [packed = true];

After recompiling I no longer get the error (I now get it in GymLevelSettings :laughing: - proto).

Does anyone here know whether it can be turned on globally, or would have to edit every occurrence. Maybe turning it on globally would break things that must not be packed on the other hand. I'd like to try though.

I will now continue to add [packed = true] until the posted rpc works, but these changes should be in AeonLucids repo I think.

I'm using libprotoc 3.0.0 for compiling if this is of any matter.

Ni42 commented 8 years ago

I guess the compiler should just assume packed = true for the relevant types as the default value. But I have no idea where this is supposed to be done. XD

Ni42 commented 8 years ago

@DrDelay I wonder if you could try changing the false to true here: https://github.com/protobuf-php/protobuf-plugin/blob/master/src/Generator/Message/ReadFieldStatementGenerator.php#L67

And trying out what happens... technically this should assume packed=true by default, I can't test it right now, because this refuses to work on windows (saw the bug report...) :)

Droeftoeter commented 8 years ago

Seems to be fixed with https://github.com/protobuf-php/protobuf-plugin/pull/4

Ni42 commented 8 years ago

Yesh, you're doing better work from holiday than I am from home xD

Also I think this is not working, either, is it? https://developers.google.com/protocol-buffers/docs/encoding#packed

Protocol buffer parsers must be able to parse repeated fields that were compiled as packed as if they were not packed, and vice versa.

Droeftoeter commented 8 years ago

Hmm, that's an interesting spec. It might be better to make it do that instead of the default-packed strategy.

Ni42 commented 8 years ago

It's probably not so important for us here, since Niantic seems to follow the default settings of packing them now, but in general it might be good for the plugin to have that.

Your PR worked for me now, and the requests are parsed correctly again.

DrDelay commented 8 years ago

I re-compiled all protos with Droeftoeter/protobuf-plugin@fea131adaf9a9b6a409917050202ee0bb0c8b101, and put the result in a branch as e6e0bf540a16c8ddef7224300ed84d3ae51f59b4.

I did not test it any further right now, but it looks good (all the Settings are updated).

iBotPeaches commented 8 years ago

Thanks @DrDelay - That branch worked fine for me. I read up on the documentation and besides the docs saying that proto3 assumes packed by default, I don't see anywhere explicitly stating [packed=true/false] on the upstream proto files being against proto3 syntax. proto2 syntax was explicit by nature, but it seems in the nature of backward compatibility it may be in the best interest to upstream all the changes of [packed=true] to POGOs repo [1].

There are plenty of places in the upstream repo where it already has packed attributes. It seems the initial parsers were very strict and required this explicit nature, so authors of various tools began submitting them until they updated their parsers (much like the above commit) to prevent the need to.

[1] - https://github.com/AeonLucid/POGOProtos

DrDelay commented 8 years ago

Should be fixed with #18. Thanks to @Droeftoeter.

Leaving this open for now as https://github.com/protobuf-php/protobuf-plugin/pull/4 is not merged, and there may be a better (protobuf spec conform) solution on how to automatically handle [packed=true/false].