godot-docs icon indicating copy to clipboard operation
godot-docs copied to clipboard

Add section on working with collections in C#

Open fractaal opened this issue 1 year ago • 5 comments

Clarified that for passing collections, use Godot's collections (or raw arrays) instead of C#'s provided collections as these currently don't work with the marshaller

fractaal avatar Aug 18 '24 04:08 fractaal

Good addition! This section definitely would have helped me when I started using Godot.

You'll probably have to squash your commits as described here, since you used the web editor and added some commits for review. You could either do so now or wait until maintainers comment on the PR.

tetrapod00 avatar Aug 18 '24 06:08 tetrapod00

Making the explanation of what types Godot can recognize more comprehensive works for me, then we can omit this new section entirely.

I still think there is some value in explicitly clarifying in this article that .NET collections won't work with the marshaller, then link to the more detailed articles regarding variant compatible types and C# collections, because this is probably going to be one of the top most visited pages in the doc if people are going to be working with C# in Godot. Leaving a small aside that lets you know about this gotcha would save a world of hurt. I know it would've helped me!

Not to mention: most of the wording in cross_language_scripting.rst is about how accessing fields and calling methods in C# from GDScript is "straightforward," so I think it's important to highlight a situation where it might not be as straightforward, as it's not an outrageous logical leap to further assume that "If things really are 'straightforward,' then I must be able to just use System.Collections!"

For instance, I've found at least just one other dev experiencing the same problem (using List\<int\>, which, as discussed, won't be seen by GDScript).

fractaal avatar Aug 19 '24 07:08 fractaal

I still think there is some value in explicitly clarifying in this article that .NET collections won't work with the marshaller, then link to the more detailed articles regarding variant compatible types and C# collections

Sure, I think it's good to add those links. But I don't think it's necessary to single out collections, since they work the same way as any other type.

most of the wording in cross_language_scripting.rst is about how accessing fields and calling methods in C# from GDScript is "straightforward,"

I agree and in most of the C# documentation we try to clarify that only Variant-compatible types work. This page could be updated to be more clear about that, but I don't think we should duplicate information that is already available in other pages since they can go out of sync.

raulsntos avatar Aug 20 '24 02:08 raulsntos

FWIW that page dates back to 3.x where the marshaller was more lenient with conversions, hence how it is worded. It has only been vaguely updated since.

paulloz avatar Aug 24 '24 07:08 paulloz

Thanks for your guys' input! The collections thing might just be me, they just live in a "different headspace" so to speak - so a .NET collection with Variant compatible types could be assumed to be handled even though it isn't. Something with the type in the collection as opposed to the collection itself being Variant compatible.

We all agree on clarifying that only Variant compatible types work on this page, which can include the quick aside that this includes stuff like Collections and then lead to the more comprehensive pages talking about these topics. I don't think this is duplicating information, just makes the docs more practical to read as it addresses a common use case, and is a good pointer to the other pages.

I'm adamant to have that in as otherwise it might not be immediately apparent, and other devs might go down the same rabbit hole I did. But if it ultimately doesn't align with the writing style of the Godot Docs, I'll defer to your input. Thanks again!

fractaal avatar Sep 15 '24 22:09 fractaal

Closing this PR per raulsntos' review

skyace65 avatar Apr 07 '25 00:04 skyace65