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

Add documentation about .gdextension file #9383

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paddy-exe
Copy link
Contributor

@paddy-exe paddy-exe commented May 15, 2024

This is a PR in the effort to improve the GDExtension docs for users

Adds details how to configure the .gdextension file and aids as a quick reference guide to add support for platforms

@paddy-exe paddy-exe added content:new page Issues and PRs related to creation of new documentation pages for new or undocumented features area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels May 15, 2024
@paddy-exe paddy-exe requested a review from dsnopek May 15, 2024 23:55
@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from 2b73f33 to f567fa1 Compare May 15, 2024 23:57
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great!

Only a few notes, mostly small wording changes for clarity.

tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from b47f96d to 90e7155 Compare May 17, 2024 19:32
@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from 90e7155 to ac065d9 Compare May 17, 2024 20:19
@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from ac065d9 to 91b93c8 Compare May 19, 2024 00:47
@paddy-exe paddy-exe marked this pull request as ready for review May 19, 2024 00:49
@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from 91b93c8 to cea6ef0 Compare May 27, 2024 15:31
@paddy-exe
Copy link
Contributor Author

Thanks for the feedback! Should be good now

@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from cea6ef0 to ecc21c6 Compare May 29, 2024 14:06
@paddy-exe
Copy link
Contributor Author

Updated to feedback, added another sections about icons support and removed the icons section from the Example page as it makes no sense to have it double

Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is looking very nice :-)

I just have a few suggestions, mostly small wording changes for clarity.

tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
Comment on lines 29 to 30
| **reloadable** | Reloads the extension upon recompilation. Reloading is not supported for every exposed class. |
| | This flag should be mainly used for developing or debugging an extension. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is meant by "Reloading is not supported for every exposed class"? It should work for any class, although, some can have additional considerations, for example, editor plugins.

It may be worth pointing out that not all GDExtension bindings support it? We could mention that godot-cpp for Godot 4.2+ does - I'm not sure it makes sense trying to list other bindings, because that list will always be growing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if it is already supported for every class especially the editor plugins classes due to how they are handled with reloading. If are you certain then I will change it. I would only list the official godot-cpp support as we other language bindings can say themselves if they support reload support or not

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated now. Let me know what you think

| **reloadable** | Reloads the extension upon recompilation. Reloading is not supported for every exposed class. |
| | This flag should be mainly used for developing or debugging an extension. |
+-------------------------------+------------------------------------------------------------------------------------------------------+
| **android_aar_plugin** | The GDExtension is part of a :ref:`v2 Android plugin <doc_android_plugin>`. During export this flag |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear from this description that this is a boolean property. Perhaps we should be listing the type for each property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a Type section in the table and set either String or Boolean in there

tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
tutorials/scripting/gdextension/gdextension_file.rst Outdated Show resolved Hide resolved
@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from ecc21c6 to 0dda1da Compare June 3, 2024 15:17
Update tutorials/scripting/gdextension/gdextension_file.rst
@paddy-exe paddy-exe force-pushed the gdextension-file-quick-reference branch from 0dda1da to 7695474 Compare June 4, 2024 10:54
@paddy-exe paddy-exe requested a review from dsnopek June 4, 2024 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation content:new page Issues and PRs related to creation of new documentation pages for new or undocumented features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants