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

Remove generate_protobuf_stubs.sh #190

Closed
ftomassetti opened this issue Nov 8, 2015 · 13 comments
Closed

Remove generate_protobuf_stubs.sh #190

ftomassetti opened this issue Nov 8, 2015 · 13 comments
Milestone

Comments

@ftomassetti
Copy link
Member

We have already another file to generate protobuf stubs inside the worldengine directory

@tcld
Copy link
Contributor

tcld commented Nov 8, 2015

Do any of those work properly? I have never managed to generate protobuf/World_pb2.py successfully, so I wasn't able to add variables to class World.

@ftomassetti
Copy link
Member Author

for me the script inside the worldengine directory works

@tcld
Copy link
Contributor

tcld commented Nov 8, 2015

This one?
So I have to manually edit World.proto and it'll work? I don't see how it would generate World_pb2.py.

@ftomassetti
Copy link
Member Author

yes, that one. You can try deleting the existing World_pb2.py and it should regenerate it.

@tcld
Copy link
Contributor

tcld commented Nov 9, 2015

Ok, a new file is indeed created. But it is using the method unicode() which for me (Python 3) causes NameError: name 'unicode' is not defined.

I already asked here #188 about how to regenerate a proper, working World_pb2.py. I still don't know how the current one was generated - maybe it can only be created under Python 2? Maybe it was manually edited afterwards?

@psi29a
Copy link
Member

psi29a commented Nov 9, 2015

protobuf3 should create the new file for us, on python2 or 3 and should work on both. Unless there has been a regression somewhere?

@tcld
Copy link
Contributor

tcld commented Nov 9, 2015

I remember when you generated the current file - I got the same broken result back then as I do now. So I don't think it is a regression; I think that either you are generating it differently than me or the problem is related to Python 3.

@ftomassetti
Copy link
Member Author

protoc --version tells you 3.0.0a3, right?

@tcld
Copy link
Contributor

tcld commented Nov 9, 2015

No, it doesn't. I didn't know that was independent of the Python-installed protobuf. I'll try to update that and try again!

EDIT: That is probably the correct fix, even though I'll have to figure out how to update protoc on my system. Thanks for the hint!

@tcld
Copy link
Contributor

tcld commented Nov 9, 2015

I compiled a new version of Protobuf and everything works now - thanks for the hint, @ftomassetti .
The updating_protobuf_format.sh-script works fine, too. The output slightly changed but so far that doesn't seem to be a problem.

I had to add syntax = "proto2"; as the very first line of World.proto, however. (I assume "proto3" would be fine, too, but I didn't try).

So yes, it seems that generate_protobuf_stubs.sh should be removed - and World.proto slightly updated to keep things in working order. I'll have to do this for PR #188, so if you aren't in a hurry and see a chance of that PR being merged, you could just wait a little bit.

@ftomassetti
Copy link
Member Author

No hurry, thanks for looking into this!

@tcld
Copy link
Contributor

tcld commented Nov 16, 2015

I think this can be closed. The file has been removed in #189 .

@psi29a psi29a added this to the v0.19.0 milestone Nov 17, 2015
@psi29a
Copy link
Member

psi29a commented Nov 17, 2015

roger that.

@psi29a psi29a closed this as completed Nov 17, 2015
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

No branches or pull requests

3 participants