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

Update GDScript examples - Step By Step: Scripting #3948

Conversation

samjabrahams
Copy link
Contributor

Howdy all,

This is my first contribution to the Godot community; excited to learn more about the project over time. This is a relatively small change, but I wanted to ask a few questions:

  • Does it make sense to only update the GDScript version of these code snippets? The concern would be that it becomes harder to track that the C# version needs to be updated.
  • Tied with the above; if we're happy to include this change without updating the C# version, should I add some form of TODO or FIXME? Not sure what the current protocols/tracking mechanisms we use are.
  • Before opening this PR, I looked for issues related to updating documentation around the new Callable class, but didn't find one. I'm not sure if these little piecemeal diffs are how the maintainers like to operate, or if there are plans for a larger unified effort.

"Regular" PR message follows. Thanks!


In Godot 4.0, the connect() function no longer takes an object and a string name of a function, but rather the new Callable type. Following the current instructions leads to error messages.

This updates this page to make the GDScript options.

Unfortunately, I don't have C# development set up on my machine, so I have not updated the C# examples as I'm currently unable to validate that they work as intended.

In Godot 4.0, the connect() function no longer takes an object and
a string name of a function, but rather the new Callable type. Following
the old instructions leads to errors.

Unfortunately, I don't have C# development set up on my machine, so I
have not updated the C# examples as I'm currently unable to validate that
they work as intended.
@Calinou

This comment has been minimized.

@samjabrahams
Copy link
Contributor Author

@Calinou good to know; thanks for the context. For my knowledge: I don't see this particular change applied to the step_by_step/scripting.rst guide applied in that PR. Is that something that will get autogenerated, or is the plan for that PR to keep adding changes until it's basically complete?

@Calinou
Copy link
Member

Calinou commented Sep 1, 2020

@samjabrahams Nevermind, I thought you were modifying the GDScript basics page. This pull request seems fine to me then.

@NathanLovato
Copy link
Contributor

Thanks for your contribution. I'll merge but note I'll be rewriting the step by step guide in the coming months. So any page there is subject to big changes or being replaced entirely in the near future.

@NathanLovato NathanLovato merged commit 512b074 into godotengine:master Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants