Skip to content

Fix proto for rust march update #77

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

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

Conversation

JulianMa
Copy link

@JulianMa JulianMa commented Mar 7, 2025

The recent rust update broke compatibility with the existing proto file.

The absence of certain fields when they have their default value, makes it seen, like FP switched to
proto3:

queuedPlayers: 0 -> field not present (https://github.com/alexemanuelol/rustplusplus/issues/410)
currencyIsBlueprint: false -> field not present (https://github.com/alexemanuelol/rustplusplus/issues/409)
Monument.x: 0 -> field not present (https://github.com/alexemanuelol/rustplusplus/issues/408)

In Proto3, missing fields in the response, which are required by proto definitions, are treated as they have their default values set. So receiving (queuedPlayers missing):

AppInfo {
    name: 'Random Server',
    headerImage: '...',
    url: '...',
    map: 'Procedural Map',
    mapSize: 4250,
    wipeTime: 123123,
    players: 100,
    maxPlayers: 120,
    seed: 123123,
    logoImage: '',
    nexus: '',
    nexusZone: ''
  }

will be evaluated as if queuedPlayers: 0.

By simple migrating the proto file, every message i received so far could be parsed without issues.
This just removed the required annotation on each field + sets proto3 as version.
WARNING: Enums are required to be 0-based in proto3. I wasnt sure how facepunch is handling this, so i for now introduced a RESERVED enum value for AppEntityType and EntityType. This is not tested yet, and this can break stuff, when facepunch decided to move the existing enums from

enum AppEntityType {
  Switch = 1;
  Alarm = 2;
  StorageMonitor = 3;
}

to

enum AppEntityType {
  Switch = 0;
  Alarm = 1;
  StorageMonitor = 2;
}

Just comment when your alarm is acting like an switch :)

@JulianMa JulianMa force-pushed the migrate-to-protobuf-3 branch 2 times, most recently from 3272f17 to 3e1cecb Compare March 7, 2025 10:45
The absence of certain fields when they have their default value makes it seen, like FP switched to
 proto3. This just removed the required annotation on each field. Also: Enums are required to be 0-based. I wasnt sure how facepunch is handling this, so i for now introduced a RESERVED enum value for AppEntityType and EntityType. This is not tested yet, but only an early fix to show whats broken in general.
@JulianMa JulianMa force-pushed the migrate-to-protobuf-3 branch from 3e1cecb to 9f87ea1 Compare March 7, 2025 10:47
@JulianMa
Copy link
Author

JulianMa commented Mar 7, 2025

will test later if Switch equals 0 or 1 in the actual server response

@JulianMa JulianMa marked this pull request as draft March 7, 2025 11:25
@JulianMa
Copy link
Author

JulianMa commented Mar 7, 2025

Can confirm, switches work

@JulianMa JulianMa marked this pull request as ready for review March 7, 2025 15:22
@JulianMa JulianMa changed the title Migrate to proto3 (Fix for rust update) Fix proto for rust march update Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant