Improve C# code sample for saving nodes #10394
Conversation
|
It's worth noting that it was previously considered to use interfaces for this exact page in an earlier issue #6622 (comment):
|
tetrapod00
added
enhancement
topic:dotnet
area:manual
|
Interestingly, from the linked issue, this was also one of my concerns when writing this PR 😅 I'm glad it isn't just me.
|
|
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 |
|
cc @raulsntos |
There was a problem hiding this comment.
In general I'm in favor of making C# examples more idiomatic but, as mentioned in the linked issue (#6622 (comment)), since the text around the code examples is common to all scripting languages we have to be careful that it still makes sense. I'm also a bit concerned about adding important information directly in the code since it can't be translated.
We usually just translate the GDScript examples directly into C# which is easier to maintain. For example, if the GDScript example gets updated, the diff when comparing with the C# example will be smaller. If we want to take a different approach, we may need to be a bit more active maintaining the C# docs so I'd like to hear what other contributors think (feel free to continue this conversation on #6622).
| // ISaveable.cs | ||
| // Define an interface to be used by the overarching Save() method | ||
| // which the Node you wish to persist will need to implement. | ||
| public interface ISaveable | ||
| { | ||
| public Godot.Collections.Dictionary<string, Variant> Save(); | ||
| } |
There was a problem hiding this comment.
I don't know if this is clear enough, since we never explicitly show the Node implementing the interface. Also, comments in code can't be translated so that may make the C# code less accessible.
| // Check the node has a save function. | ||
| if (!saveNode.HasMethod("Save")) | ||
| // Check the node implements the expected interface | ||
| if (saveNode is ISaveable saveableNode) |
There was a problem hiding this comment.
You could make the diff smaller by inverting the condition with is not and keeping the early continue as before.
| if (saveNode is ISaveable saveableNode) | |
| if (saveNode is not ISaveable saveableNode) |
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.