-
Notifications
You must be signed in to change notification settings - Fork 95
Add an example class to the project. #87
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
Conversation
0649bae to
1808be8
Compare
|
I forget the details of the discussion, but I recall when @paddy-exe's template was moved to |
Yep I remember that discussion as you explained David. The reasoning behind this as far as I can recall was that it was easier to just set it up instead of having to rewrite/delete the old boilerplate code. Also the template is set up for people familiar with the system and specifically not for newcomers. |
|
Interesting! |
unvermuthet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! Covers some common things. While I find the example scene a bit superfluous, I do like that it prints something on startup. When dsnopek was testing #78, he had to write a little script to validate that the extension was being loaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include an example of _notification and NOTIFICATION_READY in the example class. For example: https://github.com/godotengine/godot/pull/90979/files#diff-8611ca838946e49aa582ca85fe561a8803577de7f21e6b1f0a88b01015d16b4f
@Calinou's comment on my PR: "The example extension code should probably print something in its constructor (or NOTIFICATION_READY), so you see it having an effect right away when it's added to the scene tree."
|
Discussed at the GDExtension meeting and the folks present were all open to including a simple example to help folks get started. One caveat that was brought up was that this could lead to a "slippery slope" with regard to adding more and more things to the example. To avoid this, we don't want to expand the example beyond what is here. @akien-mga From our recollection, it was you that requested that the original example class was removed when the "godot-cpp-template" project was created, but we're not sure where that conversation took place. Do you have any thoughts about this? |
Personally, I don't think this is necessary. I think what we should just have the bare minimum here. What we really need is a collection of demo projects, similar to the godot-demo-projects repo but for projects using godot-cpp, where we can show real full examples of various use cases. |
Personally I consider that a template is something that should be ready to use, without having to first delete unwanted demo content. Any project started from this template would need to delete Demos to show how to do various common tasks are great, but they should not be part of the template and brought along every time someone starts a new project. They should live in their own repository which people can reference alongside the documentation to know how to implement stuff (possibly copying some of it to their template-generated project if they want). Still, if there's otherwise consensus to add this here, this isn't a hill I would die on. The example is minimal and maybe people won't use this template frequently enough that having to delete/rename a demo class would be a bother. |
Personally i find it easier to edit a class than to add a new one from scratch. Most of the boilerplate is the same, just the name changes :)
Totally agreed, the idea is just to have something so if you're new you have something to test. |
Calinou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me. Remember to apply the suggestion about the p_variant parameter name though.
|
All done! |
|
Merged thank you! Also thanks everyone for the reviews and discussion! |
When you first clone
godot-cpp-template, you may still have no idea what to do next, because the gdextension doesn't actually do anything right now. This may be additionally confusing because you may not know how to check your GDExtension is even loading.While the docs explain how to create and register classes, people may not look there when starting a project. It's may be easier to have some boilerplate for it ready and get right to programming C++.
This PR adds a minimal class for example purposes. I don't think we should try to cover a lot of functionality with it, but a single registered function and some docs is probably enough for people to kick off their project from.