Godot: [Mono] Consider moving Godot data structures to a new namespace to avoid class name collisions

Created on 30 Jul 2018  路  12Comments  路  Source: godotengine/godot

Godot version:

3.0.6

OS/device including version:

Irrelevant

Issue description:

Currently, Godot's C# implementation of its Dictionary class collides with the dictionary class in System.Collections.Generic. Often, both the Godot and Systems.Collections.Generic namespaces must be used, which requires more specificity in the code when declaring dictionary types.

Perhaps Godot data structures could be moved to a namespace like Godot.Collections to avoid such collisions.

Steps to reproduce:
Irrelevant

Minimal reproduction project:

Irrelevant

Example error

error CS0104: 'Dictionary<,>' is an ambiguous reference between 'Godot.Dictionary<TKey, TValue>' and 'System.Collections.Generic.Dictionary<TKey, TValue>'
enhancement mono

Most helpful comment

We could move Array and Dictionary to Godot.Collections.

All 12 comments

(cc: @neikeq)

Yeah this is going to break a ton of large projects forcing them to go in and create an alias to the collections directive which is going to be tedious.

I would suggest implementing the rest of the System.Collections.Generic datastructures in the C# bindings also. This way there is no reason to import System.Collections.Generic at all. There aren't all that many and we can add the serialization/deserialization stuff to all of them.

Aliases are a pain to use in the meantime, by the way. C# doesn't allow for generic namespace aliases and instead requires you to have a separate alias for each generic type:

using StringDictionary = System.Collections.Generic.Dictionary<string, string>; // oof

Not pretty if you need to use different generics with your dictionaries.

This way there is no reason to import System.Collections.Generic at all.

Are you suggesting that System.Collections.Generic be reimplemented and the replacements be recommended over it? That's really not a great idea in my opinion, LINQ depends on System.Collections.Generic.IEnumerable<T>, a number of language features use IEnumerable and IEnumerator, pretty much every C# library in existence uses the collection classes, etc. Why reinvent the wheel?

We could move Array and Dictionary to Godot.Collections.

@neikeq I think that's the best solution personally, that way opt in is easier. I'm also not opposed to TMM's suggestion to just implement all of c#'s collections w/ the serialize changes. Regardless of what you do it's early enough that not everyone will be using Godot.Dictionary to make said change.

@hpvb Performance, for one. If I need to store a list purely in my C# code, I absolutely will use System.Collections.Generic.List<T> because it will perform much better than going through Godot.

Also, can you store non-Godot.Object/marshall-able things in the Godot collections?

I would also vote against re-implementing all of System.Collections.Generic unless there's a very compelling reason to do so.

I like @neikeq 's idea to move Array and Dictionary into Godot.Collections. Even in large projects this shouldn't take but a few minutes (if that) for people to add to the top of their cs files.

This is also true for arrays; there's now a collision between Godot.Array and System.Array. By default, both System and Godot are imported. This is wreaking havoc on my C# project since nominally, an array object initialized via e.g. float[] is a System.Array. An array of a node class will complain that Godot.Array cannot be cast to a System.Array.

Also, can you store non-Godot.Object/marshall-able things in the Godot collections?

No, it's not possible. Array stores Variant elements. Variant elements only understand of Godot types. If you want to store a non-Godot.Object in a Variant, you need to wrap it in a Godot.Object. I'm considering making this automatic, with some pool to avoid much overhead, or actually adding MonoObject as a Variant type.

Is it possible to get this in 3.0.7 @neikeq or if it's as simple as adding a namespace to Array.cs and Dictionary.cs I'll make a PR.

I would appreciate this fix. Currently I alias my dictionary types since they're just too long to type. If I didn't need to prepend "System.Collections.Generic" then that would be nice.

using IntDict = System.Collections.Generic.Dictionary<int, int>;

@aaronfranke https://github.com/godotengine/godot/pull/21423 should be available with 3.0.7, neikeq said he'd merge this when he gets a chance. Most of those changes are his :)

Was this page helpful?
0 / 5 - 0 ratings