Mindwerks / worldengine

World generator using simulation of plates, rain shadow, erosion, etc.
MIT License
981 stars 127 forks source link

Remove generate_protobuf_stubs.sh #190

Closed ftomassetti closed 8 years ago

ftomassetti commented 8 years ago

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

tcld commented 8 years ago

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 commented 8 years ago

for me the script inside the worldengine directory works

tcld commented 8 years ago

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 commented 8 years ago

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

tcld commented 8 years ago

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 https://github.com/Mindwerks/worldengine/pull/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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

protoc --version tells you 3.0.0a3, right?

tcld commented 8 years ago

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 commented 8 years ago

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 https://github.com/Mindwerks/worldengine/pull/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 commented 8 years ago

No hurry, thanks for looking into this!

tcld commented 8 years ago

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

psi29a commented 8 years ago

roger that.