Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[.NET] Fix VerifyUnion using wrong offset for typeId #8360

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

0x4261756D
Copy link

The offset was overwritten to point to the data before the typeId was set.

The offset was overwritten to point to the data before the typeId was set.
Copy link

google-cla bot commented Jul 12, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the c# label Jul 12, 2024
@0x4261756D
Copy link
Author

Oh well, too bad having a Google account is a requirement for contributing. Seems like this change won't make its way into the repo then I guess.

0x4261756D added a commit to 0x4261756D/CardGame that referenced this pull request Jul 16, 2024
This fails when trying to start a room and artwork fetching was disabled
because packets got too big (not quite sure why exactly).

Negative things about FlatBuffers:
* Verification is bugged in the upstream C# library, hence the submodule.
  * I opened a [PR](google/flatbuffers#8360) but since you
    need a Google account to have any chance of getting merged it seems like there
    is no hope for getting it in.
* Enum variants being untyped is **HIGHLY** irritating, I have already encountered
  more than one case of using `Packet` instead of `PacketT` and only catching it
  because the verifier on the other side rejects it.
* The verifier does not tell you what failed. As brittle as this whole library is
  just spitting out a "no" is entirely unhelpful.
* Many parts of the C# library seem weird. It is hard to compare them to their
  counterparts in other languages since they differ quite a lot but on several
  occasions it was unclear to me whether *I* did something wrong or the library
  is just buggy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant