Skip to content

Conversation

@michaelgundlach
Copy link
Contributor

Clarify the confusing text and code sample around property initialization.

The previous text implied (at least to me) that there was something special in Godot called an "init assignment value", and that there was special logic determining which kind of assignment was used. In reality, nothing special is happening: the property is initialized, then the constructor may set the value, then exports may as well.

I also harmonized the Godot and C# code samples, rather than expecting the user to read both in order.

Frankly I almost feel like this would be clearer if the entire section were simply deleted, but I wasn't sure if that would be accepted.

Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

Thanks for contributing to the documentation!

// _test is initialized to "one".
private string _test = "one";

// Changing the value from the inspector does trigger the setter in C#.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is important because it lets users know that property setters will be executed by the inspector. It's true that the previous message seems to assume that you've read the GDScript example before, so it could be reworded but I don't think it should be removed entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This data is moved to line 220. I put discussion of @exports at the bottom of the file in both languages so that the order of value assignment matches the order you read the example.

Copy link
Member

Choose a reason for hiding this comment

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

I see, I guess it's fine. I still find it a bit weird that information about the exported property is so far from the property itself.

@raulsntos raulsntos added enhancement topic:dotnet topic:gdscript area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Oct 24, 2023
@michaelgundlach michaelgundlach force-pushed the patch-1 branch 2 times, most recently from 231879c to 6d13fcd Compare October 24, 2023 16:22
@michaelgundlach michaelgundlach changed the title Update godot_notifications.rst Clarify confusion around property initialization Oct 29, 2023
@mhilbrunner
Copy link
Member

ping @raulsntos :)

Comment on lines 167 to 173
that code should execute in ``_init()``. Other property or SceneTree-independent
initializations should also run here.

Scripts have three types of property assignments that can occur during
instantiation:
``_init()`` triggers before ``_enter_tree()`` or ``_ready()``, but after a script
Copy link
Member

Choose a reason for hiding this comment

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

The new text mentions _init so we may want to add a note for C# users. In C#, the _init method doesn't exist and instead users should use the regular class constructor.

The C# constructor will also execute before any of the engine callbacks (_EnterTree, _Ready, etc.) and after what's being referred to here as Initial value assignment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, adding a note that C#'s _init() equivalent is the constructor.

The previous text implied (at least to me) that there was something
special in Godot called an "init assignment value", and that there was
special logic determining which kind of assignment was used.  In
reality, nothing special is happening: the property is initialized, then
the constructor may set the value, then exports may as well.

I also harmonized the Godot and C# code samples, rather than expecting
the user to read both in order.

Frankly I almost feel like this would be clearer if the entire section
were simply deleted, but I wasn't sure if that would be accepted.
Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

LGTM.

@skyace65 skyace65 merged commit d82940e into godotengine:master Nov 24, 2023
@skyace65
Copy link
Contributor

Thanks!

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 enhancement topic:dotnet topic:gdscript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants