-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[Announcement] [Gdscript] Godot 4.x Client Work in Progress #13719
Comments
It's up to you and I think the community gets used to mustache. |
Would this help? https://github.com/OpenAPITools/openapi-generator/wiki/Integration-Tests |
The backbone is here and as I'm getting slowly more proficient in OAS code gen, it's time to delve into tests. |
The doc states:
I can't find any Also, for integration testing, I think I can make a single (or more later as needed) GDScript test file and run it from the CLI, and return an error code I can wrap the call(s) into a shell script for convenience. Where should I put such files ? |
I'm glad to announce that the integration test can create a user, log in, and create a pet. I'm stuck, thoughI'm now using that test to do some refactoring and exploration into namespaces. I can rather easily prefix and suffix the Api and Models with their already-defined parameters. I'm not doing anything, it _just works_®. Yet I have some core files (base class, error) that I want to be able to customize the same way, using for example generatorName: gdscript
outputDir: samples/client/petstore/gdscript
inputSpec: modules/openapi-generator/src/test/resources/3_0/petstore.yaml
templateDir: modules/openapi-generator/src/main/resources/gdscript
additionalProperties:
hideGenerationTimestamp: "true"
apiNamePrefix: Demo
modelNamePrefix: Demo
coreNamePrefix: Demo The property
I want to change the filename when defining my supportingFiles.add(new SupportingFile("ApiBee.handlebars", "core", toCoreFilename("ApiBee") + ".gd")); The method This probably ties into Any lights on these would be appreciated. @wing328 ? |
It was too baffling ; I should have noticed my confusion earlier, but it's only after re-reading my message above that I thought of the wrong assumption I was making. public static final String CORE_NAME_PREFIX = "coreNamePréfix"; When I wrote this line, I checked against presence in the template, and io and behold the variable was there, so I went merrily on my way.
Got stuck way too long on a sneaky diacritic, but at least I learned a lot about the internals of the generator along the way 🦊 ! |
I'm going to take some time and consume a real-world OAS with this, before I carry on and freeze things for MR. Might take a few months.
|
@Goutte thanks for your work on this. Can you please PM via Slack me when you've time? https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g |
At least Godot 3 has a style guide, that is based on how the c++ code is formatted. I don't think it changed for 4, would be the best to follow this. https://www.reddit.com/r/godot/comments/yngda3/gdstyle_naming_convention_and_code_order_cheat/ |
Any updates on this @Goutte ? |
Hello, I found a bug with generating default values for strings. More details: Goutte#1 |
@Goutte can you please submit a PR so that we can incorporate this new generator into the official repo so that more Godot users can benefit from this and submit PRs to further enhance it? |
@wing328 Shall we implement more features or we can merge current WIP? I started using this generator for my pet project. So I may found more problem in future. And may address them. |
Actually there is one more bug, which I haven't fix for now. Just modified generated code. Godot 4 was under active development. Now they seem to freeze changes. (At least they mark some unstable api which they are going to modify in future releases) The method And this is a latest documentation: https://docs.godotengine.org/en/stable/classes/class_httpclient.html#class-httpclient-method-connect-to-host Basically as I understood they merge |
Thanks @saatsazov for the bug finds ! I'll look into wrapping this up for a PR now, it will help me procrastinate the writing of a CV. 🙄 |
@saatsazov I've added support for TLS and some underscores to method names, to mark them as protected. It's here : Goutte#2 It's a breaking change ; there might be more before the MR, since I frown at the current API. (even the |
Hello @Goutte, No worries about breaking changes. My project is not on a stage to worry about that. But thanks for worried mentioning breaking changes. I actually started doing some quick and dirty fixes in my fork. One of the biggest problem I encountered is connected with connection. When you start doing request. the whole game is very laggy. So what I did is started a separate thread. I suppose it should be some better way to do so. I tracked problem to this loop https://github.com/Goutte/openapi-generator/blob/13d3c5c79d1f2a4bf4a464a00a59d1b9d68f1793/modules/openapi-generator/src/main/resources/gdscript/ApiBee.handlebars#L100 The delay seems to block main thread. So as a quick solution I added a thread. As you mentioned in a comments here https://github.com/Goutte/openapi-generator/blob/13d3c5c79d1f2a4bf4a464a00a59d1b9d68f1793/modules/openapi-generator/src/main/resources/gdscript/ApiBee.handlebars#L64 there is no such thing as idle frame. So we should think how to approach this. I may provide an example project later if you want. |
Good point ! Whatever the Inline objects hurdleTLS works, but now I need to support inline objects like these: /authentication_token:
post:
operationId: login_check_post
tags:
- 'Login Check'
responses:
200:
description: 'User token created'
content:
application/json:
schema:
type: object
properties:
token: { readOnly: true, type: string, nullable: false }
required:
- token
summary: 'Creates a user token.'
requestBody:
description: 'The login data'
content:
application/json:
schema:
type: object
properties:
username:
type: string
nullable: false
password:
type: string
nullable: false
required:
- username
- password
required: true
parameters: []
I looked at how typescript did it in |
At the moment I can't say how would be better to work with async. So we can address this issues later when we have more details. Speaking of inline object I use this generator for PHP project and this is how it looks like. It generate a bunch of of classes per each inline request and response. This may mitigate the problem you mention one class per file. |
I've added rudimentary support for inline objects (request/response, mainly). Still not sure what to put in the |
Breaking
TestingI remember managing to set up a test project with a custom MainLoop or something, for automated integration testing, but it's complicated to set up as it requires Godot to run, and docker for the petstore server. Unit tests don't feel as relevant, though. |
Gdunit+docker should be fine imo. |
I'll look today into refactoring the test project (in |
I like the assertions more, to me they are more readable. Nothing else.
Executing the generated code in godot as a functional test would have value imo. That can also be launched from java if needed. |
Definitely ! So far we can register, authenticate and CRUD a monkey on the petstore. The command line is still a bit complex, but it passes (provided the dockerized petstore is running):
Breaking soonI'm currently wrapping the object sent back into the Even if we don't provide the full response analysis for now, changing the callback signature now will help us implement these features later on without breaking. |
I don't feel anyhow about it. I'll most likely just be a consumer anyways. 😅 |
Now I remember why I had not added a test engine but made a minimalist (disgusting) one. Oh well… The review of this target is going to be more painful, but I guess it's worth it ? @saatsazov Does it still lag a lot when you set So far I'm using the client in menus, but in a few months I will start using it during heavy 3D rendering as well, so I will inspect the async/threading issue more closely then. I'll let this sit until then (I fixed my email notification issue), perhaps I will fix/implement the response's character set detection, but that should not be a breaking change. Latest draft has been merged in feat-gdscript. |
Also, this warrants interest : https://github.com/D2klaas/Godot-4-HTTPManager |
I am interested in helping test out this generator. I am trying to utilize feat-gdscript via And thanks @Goutte for your work on this generator! |
@jchu231 you will need to build https://github.com/Goutte/openapi-generator/tree/feat-gdscript locally for the time being as the generator has not been merged into master. |
@Goutte , I am testing your generator (thanks for your awesome work btw!). Ran into this error when generating the first api: Looks like there is an I think either renaming |
@jchu231 Thanks ! I was not very comfortable with the |
Thanks for pushing out the fix, @Goutte !! I've been running into another issue, not sure if you've encountered this, but I am seeing some issues with Error in Godot console looks like: My Link to related code in |
I bet that by now you understand why this generator ain't a merge request yet. :) This data retrieval inner loop needs some more love. I suspect the error is raised by the call to I'll keep an eye out ; meanwhile, I invite you to experiment with that loop and break things, since the current implementation can be qualified as shenanigans more than code. I also notice that the |
I think I was able to fix the issue I was seeing! (the I had to move the call to I had a call that was failing 100% of the time (I think the larger the response body, the more likely this issue surfaces), and after this change it now works every time. My (very rough) theory is that |
@jchu231 That's great news ! You can make a MR to my branch, or have me fix this on my end. Your pick. I'll run the test-suite with the change (but I expect it to pass). |
@Goutte , do you might making this change? I'm not super comfortable with the generator codebase yet (I've just been editing the generated files), but happy to review and then I can learn how to make changes for the next time. I also tweaked the order of the code in the |
I think it's time to merge this not finished work to master of openapi. As this is the only app that exists. And also the openapi have some problems solved but I can't use directly your branch. as it still have some bugs. I actually tried to take latest master from OpenAPITools/openapi-generator and lates feat-gdscript branch. And still got some bug generating my schema. |
I had been waiting for Godot 4 to fill in the gaps regarding the Callables and typing. I appears that this is now OK with Godot 4.3 ; does anyone absolutely need support for Godot 4.0 or 4.1 ? @saatsazov ? @jchu231 ? Finished or not, you're right ; it's about time to make a MR so that collab' is easier. |
Thank you for asking @Goutte. No I don't mind to have minimum 4.3 version. |
Hi, any progress to this? |
Hello !
I started experimenting with a Godot client (
GDScript
), and albeit there's not much to show right now I wanted to write something here in case anyone else is interested in joining forces. I looked here for the keywordsGodot
andGDScript
.Rationale
Being able to quickly draw a pure
GDScript
client for any OAS would be amazing for some online games, even though Godot has its own network engine, because in HTML5 exports only the HTTP client is available (for now). Anyway, there's plenty of uses for HTTP connections to a server in games (auth, leaderboards, news, etc.).Y U NO C ?
Having to compile Godot itself and distribute it to all collaborators is no small task.
And I'd rather not have Mono if I can help it.
Design
Indent using Tabs
Godot uses tabs by default, although it is configurable.
Try not to pollute global space (with
class_name
) ?Godot does not have namespaces. (for now)
What should we add in the global namespace (via
class_name
, or perhaps singleton nodes) ?If we do decide to use
class_name
(and pollute global space) for models and api, which I suspect we'll need to get nice type defs, we need to set up for all class names a prefix that will act as a namespace.snake_case vs camelCase
I'd love some configuration but I won't bother with it until at least I have a working client.
Static typing
Godot does not allow nullable types (we're waiting for them).
This means we either skip types, or try to figure out how to replace
null
by the appropriate pseudo-null value for the given type. (""
forString
, etc.)But it becomes awkward for
int
values, where0
might not be a sensible pseudo-null.Async
In HTML5 exports, only async is available.
Since it is my personal target, I'll favor it.
Godot 4 has
Callable
s and lambda defs, but noPromise
s yet.I think the best method signature for our generated async API method endpoints would be to return a
Promise
.But if we start waiting on the promises of Godot… ;)
Therefore, for the first major version of the generator, we'll use
Callable
s in that fashion, unless you have a better idea :To Singleton or not to Singleton
I tried to avoid using Singletons (aka. Autoloads), so
XxxxApi
classes are simpleRefCounted
(plain ol' Godot Object with garbage collection), but that means eachXxxxApi
holds its own (host, port) configuration to lazily create its client when none was provided. Same goes for extra headers the user wants to send.You can create the configuration once and pass it to each API class.
State of Things
I managed to bootstrap the
gdscript
target, compile and generate the petstore sample.Now we need to fill the templates with working code, one feature at a time.
Array
s with dubious recursion supportrequired
length
, etc.)Deserialization (JSON+LD)← skipped (ouch)Serialization (XML)← skippedDeserialization (XML)← skippedI'm looking at the other language targets and throwing darts. Wish me luck !
Burning questions
Is there somewhere a docker image (or whatever) for a server with a working petstore I can use for my demo during dev ?
Should I use
handlebars
straight away or keep usingmustache
, as mildly recommended ? I'm used to (and fond of)jinja2
andTwig
(Pebble
, for Java).I made a small python script to grab the reserved keywords from Godot's XML. Since it might be useful again when updating the generator to account for changes in Godot, I'd like to keep it close. Where can I store it ? (right now it's in
modules/openapi-generator/src/main/resources/gdscript
, schmoozing with the templates)The text was updated successfully, but these errors were encountered: