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

Improve C# code sample for saving nodes #10394

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tombasche
Copy link

I noticed this code sample in the tutorial wasn't very idiomatic C# code, and there is perhaps a more type-safe way to handle saving of nodes within a project using interfaces.

@tetrapod00
Copy link
Contributor

It's worth noting that it was previously considered to use interfaces for this exact page in an earlier issue #6622 (comment):

There are plenty of examples of this in the documentation, see the Saving Games page, which uses duck typing and Godot's JSON serializer while it may better to use interfaces and one of .NET's more powerful serializers.

@tetrapod00 tetrapod00 added enhancement topic:dotnet area:manual Issues and PRs related to the Manual/Tutorials section of the documentation content:example code Issues and PRs involving code examples cherrypick:4.3 labels Dec 10, 2024
@tombasche
Copy link
Author

Interestingly, from the linked issue, this was also one of my concerns when writing this PR 😅 I'm glad it isn't just me.

though I'm worried that users may be confused as to where IHittable is coming from, since it's not a type provided by Godot or the BCL, we'll likely want to add some text explaining what it is and then that may be confusing to GDScript users.

@tetrapod00
Copy link
Contributor

You might be able to use the second codeblock (under https://docs.godotengine.org/en/latest/tutorials/io/saving_games.html#serializing) to also include the definition of ISaveable. Maybe use // comments to distinguish which file each line is in, which is often bad practice (separate codeblocks are better) but in this case since the text and codeblocks in these tutorials are designed for GDScript then adapted for C#, there's only so much you can do.

@tetrapod00 tetrapod00 requested a review from a team December 10, 2024 23:12
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 cherrypick:4.3 content:example code Issues and PRs involving code examples enhancement topic:dotnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants