oniksan / godobuf

A Google Protobuf implementation for Godot / GDScript
BSD 3-Clause "New" or "Revised" License
248 stars 34 forks source link

Add Godot 4 compatible branch #30

Open kelteseth opened 2 years ago

kelteseth commented 2 years ago

Hi, I adapted most of the API changes in my fork of godobuf. It is not ready yet to be merged but you can take a look at the changes

https://github.com/kelteseth/godobuf

Godot API Changes:

Buggy:

Not working: protobuf_core.gd line 106 and 109 error when using const with []. No idea how to solve this.

const DEFAULT_VALUES_3 = {
        [...]
    #PB_DATA_TYPE.BYTES: [],
    PB_DATA_TYPE.MESSAGE: null,
    #PB_DATA_TYPE.MAP: []
}

This next one looks similar to https://github.com/oniksan/godobuf/issues/26

Parse Error: Identifier "PROTO_VERSION" not declared in the current scope.
  res://addons/protobuf/protobuf_core.gd:533 - Parse Error: Identifier "PROTO_VERSION" not declared in the current scope.`
kelteseth commented 2 years ago

@oniksan Could you help me to fix the last few bits? :)

oniksan commented 2 years ago

@kelteseth, Hi!

Sorry, I have not studied Godot 4 and don't know what changes have been made regarding the language constructs of GDScript, therefore, I cannot answer your question now. Maybe when I migrate to stable Godot 4, I will have to figure it out. When I ported code from v2 to v3 of Godot, there were also similar incompatibility problems that were simply not solved.

Unfortunately, the Godot developers for some reason do not think about backward compatibility at all. On the one hand, this is correct - new code will be clean, but on the other hand - what to do with existing large projects.

Regarding Parse Error: Identifier "PROTO_VERSION", it's not an error, I already wrote about it in 26. Godot IDE is trying to parse data that has not yet been generated. The core file should not be parsed separately from generated code. I could change extension of the file, but then it would be inconvenient to type it in IDE, so I don't do it. This "error" does not break anything, because this file is not involved in the plugin. Text in this file only works in generated code of your program.

kelteseth commented 2 years ago

@oniksan I just tested it with the current Godot nightly from https://hugo.pro/projects/godot-builds/ and it seems that DEFAULT_VALUES_3 issue is fixed :). So it looks like only some UI design improvements are left: image

image

kelteseth commented 2 years ago

https://user-images.githubusercontent.com/1071536/170317558-5b62b9db-1803-4103-91e9-2f4acdb5298a.mp4

@oniksan should be all fixed now. Should I create a PR? https://github.com/kelteseth/godobuf/commit/7e501da486e956e0b7d9232e2b2540908568bca2

oniksan commented 2 years ago

This is wrong fix, because generated code will contain two constants: изображение Read my previous post carefully.

kelteseth commented 2 years ago

I fixed the duplicate PROTO_VERSION and ported some unit tests. Not sure how to port:

    var test_func = funcref(test, test_name[FUNC_NAME])

because Godot 4 required actual object references (not sure if this is true). @oniksan would you be willing to help me so we can merge this?

oniksan commented 2 years ago

Hi, @kelteseth

I think that before the release of stable version 4, it's not worth changing anything and people are using the stable version of Godot and want to use the appropriate stable Godobuf version. I have not yet studied the documentation on changes in GDScript for Godot 4, so I'm unlikely to be of any help now. After the stable version of Godot is released, and I will definitely look at your code. You can do PR if you want when the stable version comes out.

MadFlyFish commented 1 year ago

Hello,Godot 4.0 RC is released. Is there any plan to add Godot 4 compatible branch? Thanks.

oniksan commented 1 year ago

Hi, rather yes than no :)

cmdrk commented 1 year ago

Just wanted to 👍 this issue, now that Godot 4.0 has released.

Is there anything the community can do to contribute? Thanks!

TheDarkUndoing commented 1 year ago

I am willing to help if needed. @kelteseth does your branch still work with 4.0 stable?

kelteseth commented 1 year ago

@TheDarkUndoing I will have to test it again and probably with teste changes https://github.com/kelteseth/godobuf/pulls

kelteseth commented 1 year ago

@oniksan Godot 4 fork is now ready. Thanks to @xiyoo0812 & @cmdrk for their fixes. Should I create a PR now? Also can I update the logo to include the protobof logo with the Godot mascot? 😊

oniksan commented 1 year ago

Sorry, I don't have time yet, you can do a PR, I'll see how it will be possible.

kelteseth commented 1 year ago

@oniksan Godot 4 Object now also has as to_string function:

image

What should we name it? message_to_string?

oniksan commented 1 year ago

@kelteseth We should override the object's to_sting() if possible. Or does the to_string object's method have a different purpose?

Sammyjroberts commented 1 year ago

Can I help with porting this to gd4 somehow?

oniksan commented 1 year ago

Hi all. @Sammyjroberts New version is available. @kelteseth I couldn't wait for the pull request from you and fixed everything myself.

https://github.com/oniksan/godobuf/releases/tag/v0.6.0 If you find bug, create an issue please.