search

RoboCup-SSL / ssl-game-controller

The game controller for matches in the RoboCup Small Size league
GNU General Public License v3.0
19 stars 43 forks source link

Name Conflicts When Importing Protobuf Definitions #16

Closed jonathanlew closed 4 years ago

jonathanlew commented 4 years ago

Context: Recently, we tried importing the new protobuf definitions from this repo into our repo, namely ssl_gc_common.proto, ssl_gc_game_event.proto, ssl_gc_geometry.proto, and ssl_gc_referee_message.proto. We wanted to do this because https://github.com/RoboCup-SSL/ssl-refbox/ has been deprecated, and this repo's definitions has some updated documentation.

Problem: We encountered name conflicts between our internal Team and RobotId classes and the ones defined in ssl_gc_common.proto. This is a change from defining them inside the GameEvent message as in https://github.com/RoboCup-SSL/ssl-refbox/. We also noticed that SSL_Referee has been renamed to Referee, which might cause name conflicts in the future (but currently don't in our repo). We're curious why these changes were made as it causes problems for easily updating these protobufs in our code and other teams would probably encounter the same issues.

Suggested solution: Prefix protobuf messages with SSL_ or embed them in other message definitions, as was done in https://github.com/RoboCup-SSL/ssl-refbox/

g3force commented 4 years ago

I removed the SSL_ prefix intentionally, because it is not a valid name according to the protobuf naming convention: https://developers.google.com/protocol-buffers/docs/style#message_and_field_names

If you do not want to refactor your code, you can just rename the field in your codebase. The message and field names are not part of the API.

Name conflicts can easily be solved using packages, as the GC does with go_package and for example the TIGERs AutoRef does with package: https://github.com/TIGERs-Mannheim/AutoReferee/blob/master/modules/moduli-referee/src/main/proto/ssl_gc_referee_message.proto#L3

One difficulty are enums indeed. They must be unique outside packages as well. I think that's why the Team is in a common package. Is this an issue in your code base as well? And if so, can't you reuse the Team and/or RobotId?

jonathanlew commented 4 years ago

Thanks for the explanation. I added package to the ssl_gc protobufs and that works in our repo. For C++, package gets compiled into namespaces, so enums aren't a problem at all. For example, assuming package SSL;, the C++ type for the Team protobuf message would be SSL::Team. That fixes my problem, so closing this issue