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

Allow using generics with Variant.CreateFrom() and Variant.ConvertTo() #5762

Closed
avilches opened this issue Nov 10, 2022 · 4 comments
Closed

Comments

@avilches
Copy link

Describe the project you are working on

A C# library to create animations and effects with more complex tweens.

Describe the problem or limitation you are having in your project

I have some classes with generic to interpolate node properties, so user can create something like

var property = Property.Create<float>("position:x");

So, I have a kind of class class Property<T> where T is float (or any other type compatible with Variant). Then I use node.GetIndexed and node.SetIndexed to update the node property, but I have first to convert a T value to Variant, in order to use these methods.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

If I had a T Variant.ConvertTo<T>(Variant v) and Variant CreateFrom<T>(T v) methods, I could use in my library. And any other user in the same situation (convert variant to a generic type and the other way around) could use it too.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I have solved this with these two methods, that can be used (or improved, or ignored) to achieve it:

    public static class VariantHelper {
        public static Variant CreateFrom<T>(T value) {
            return value switch {
                bool valueBool => valueBool,
                char valueChar => valueChar,
                sbyte valueSbyte => valueSbyte,
                short valueShort => valueShort,
                int valueInt => valueInt,
                long valueLong => valueLong,
                byte valueByte => valueByte,
                ushort valueUshort => valueUshort,
                uint valueUint => valueUint,
                ulong valueUlong => valueUlong,
                float valueFloat => valueFloat,
                double valueDouble => valueDouble,
                Vector2 valueVector2 => valueVector2,
                Vector2i valueVector2i => valueVector2i,
                Rect2 valueRect2 => valueRect2,
                Rect2i valueRect2i => valueRect2i,
                Transform2D valueTransform2D => valueTransform2D,
                Vector3 valueVector3 => valueVector3,
                Vector3i valueVector3i => valueVector3i,
                Vector4 valueVector4 => valueVector4,
                Vector4i valueVector4i => valueVector4i,
                Basis valueBasis => valueBasis,
                Quaternion valueQuaternion => valueQuaternion,
                Transform3D valueTransform3D => valueTransform3D,
                Projection valueProjection => valueProjection,
                AABB valueAABB => valueAABB,
                Color valueColor => valueColor,
                Plane valuePlane => valuePlane,
                Callable valueCallable => valueCallable,
                SignalInfo valueSignalInfo => valueSignalInfo,
                byte[] valueArrayByte => valueArrayByte.AsSpan(),
                int[] valueArrayInt => valueArrayInt.AsSpan(),
                long[] valueArrayLong => valueArrayLong.AsSpan(),
                float[] valueArrayFloat => valueArrayFloat.AsSpan(),
                double[] valueArrayDouble => valueArrayDouble.AsSpan(),
                string[] valueArrayString => valueArrayString.AsSpan(),
                Vector2[] valueArrayVector2 => valueArrayVector2.AsSpan(),
                Vector3[] valueArrayVector3 => valueArrayVector3.AsSpan(),
                Color[] valueArrayColor => valueArrayColor.AsSpan(),
                Object[] valueObject => valueObject, 
                StringName[] valueArrayStringName => valueArrayStringName.AsSpan(), 
                NodePath[] valueArrayNodePath => valueArrayNodePath.AsSpan(), 
                RID[] valueArrayRid => valueArrayRid.AsSpan(), 
                Object valueObject => valueObject, 
                StringName valueStringName => valueStringName, 
                NodePath valueNodePath => valueNodePath, 
                RID valueRid => valueRid, 
                Dictionary valueDictionary => valueDictionary, 
                Array valueArray => valueArray, 
                _ => throw new Exception("Unknown variant")
            };
        }                

        public static T ConvertTo<T>(Variant value) {
            var t = typeof(T);
            if (t == typeof(bool)) return (T)(object)value.AsBool();
            if (t == typeof(char)) return (T)(object)value.AsChar();
            if (t == typeof(sbyte)) return (T)(object)value.AsSByte();
            if (t == typeof(short)) return (T)(object)value.AsInt16();
            if (t == typeof(int)) return (T)(object)value.AsInt32();
            if (t == typeof(long)) return (T)(object)value.AsInt64();
            if (t == typeof(byte)) return (T)(object)value.AsByte();
            if (t == typeof(ushort)) return (T)(object)value.AsUInt16();
            if (t == typeof(uint)) return (T)(object)value.AsUInt32();
            if (t == typeof(ulong)) return (T)(object)value.AsUInt64();
            if (t == typeof(float)) return (T)(object)value.AsSingle();
            if (t == typeof(double)) return (T)(object)value.AsDouble();
            if (t == typeof(string)) return (T)(object)value.AsString();
            if (t == typeof(Vector2)) return (T)(object)value.AsVector2();
            if (t == typeof(Vector2i)) return (T)(object)value.AsVector2i();
            if (t == typeof(Rect2)) return (T)(object)value.AsRect2();
            if (t == typeof(Rect2i)) return (T)(object)value.AsRect2i();
            if (t == typeof(Transform2D)) return (T)(object)value.AsTransform2D();
            if (t == typeof(Vector3)) return (T)(object)value.AsVector3();
            if (t == typeof(Vector3i)) return (T)(object)value.AsVector3i();
            if (t == typeof(Basis)) return (T)(object)value.AsBasis();
            if (t == typeof(Quaternion)) return (T)(object)value.AsQuaternion();
            if (t == typeof(Transform3D)) return (T)(object)value.AsTransform3D();
            if (t == typeof(Vector4)) return (T)(object)value.AsVector4();
            if (t == typeof(Vector4i)) return (T)(object)value.AsVector4i();
            if (t == typeof(Projection)) return (T)(object)value.AsProjection();
            if (t == typeof(AABB)) return (T)(object)value.AsAABB();
            if (t == typeof(Color)) return (T)(object)value.AsColor();
            if (t == typeof(Plane)) return (T)(object)value.AsPlane();
            if (t == typeof(Callable)) return (T)(object)value.AsCallable();
            if (t == typeof(SignalInfo)) return (T)(object)value.AsSignalInfo();
            if (t == typeof(Span<byte>)) return (T)(object)value.AsByteArray();
            if (t == typeof(Span<int>)) return (T)(object)value.AsInt32Array();
            if (t == typeof(Span<long>)) return (T)(object)value.AsInt64Array();
            if (t == typeof(Span<float>)) return (T)(object)value.AsFloat32Array();
            if (t == typeof(Span<double>)) return (T)(object)value.AsFloat64Array();
            if (t == typeof(Span<string>)) return (T)(object)value.AsStringArray();
            if (t == typeof(Span<Vector2>)) return (T)(object)value.AsVector2Array();
            if (t == typeof(Span<Vector3>)) return (T)(object)value.AsVector3Array();
            if (t == typeof(Span<Color>)) return (T)(object)value.AsColorArray();
            if (t == typeof(Object[])) return (T)(object)value.AsGodotObjectArray<Object>();
            // if (t == typeof(Dictionary<TKey, TValue>>) return (T)(object)value.AsGodotDictionary<TKey, TValue>());
            // if (t == typeof(Godot.Collections.Array<T>>)) return (T)(object)value.AsGodotArray();
            if (t == typeof(Span<StringName>)) return (T)(object)value.AsSystemArrayOfStringName();
            if (t == typeof(Span<NodePath>)) return (T)(object)value.AsSystemArrayOfNodePath();
            if (t == typeof(Span<RID>)) return (T)(object)value.AsSystemArrayOfRID();
            if (t == typeof(Object)) return (T)(object)value.AsGodotObject();
            if (t == typeof(StringName)) return (T)(object)value.AsStringName();
            if (t == typeof(NodePath)) return (T)(object)value.AsNodePath();
            if (t == typeof(RID)) return (T)(object)value.AsRID();
            if (t == typeof(Dictionary)) return (T)(object)value.AsGodotDictionary();
            if (t == typeof(Array)) return (T)(object)value.AsGodotArray();
            throw new Exception("Unknown variant");
        }                

If this enhancement will not be used often, can it be worked around with a few lines of script?

I already have the methods working, but I think this can be useful for all the users.

Is there a reason why this should be core and not an add-on in the asset library?

It looks to my a lack in the Variant class than a extension

@avilches
Copy link
Author

The ProjectSettings static class and ConfigFile could be improved with these new methods, if they are finally added with something similar to this:

        public static T GetProjectSetting<T>(string key, T defaultValue) {
            if (!ProjectSettings.HasSetting(key)) return defaultValue;
            var variant = ProjectSettings.GetSetting(key);
            return VariantHelper.ConvertTo<T>(variant);
        }

        public static void SetProjectSetting<T>(string key, T value) {
            var variant = VariantHelper.CreateFrom(value);
            ProjectSettings.SetSetting(key, variant);
        }

        public static void SetTypedValue<T>(this ConfigFile configFile, string section, string key, T value) {
            var variantValue = VariantHelper.CreateFrom(value);
            configFile.SetValue(section, key, variantValue);
        }
        public static T GetValue<T>(this ConfigFile configFile, string section, string key, T @default = default) {
            var variantDefault = VariantHelper.CreateFrom(@default);
            var variantValue = configFile.GetValue(section, key, variantDefault);
            return VariantHelper.ConvertTo<T>(variantValue);
        }

@raulsntos
Copy link
Member

Variant only allows a few types and they can already be implicitly converted so I don't see the benefit of allowing T to be used to set/get a project setting. You should already know what type the setting is, I can't think of a use case where you want to get a specific setting but don't know its type.

Currently, because all Variant compatible types are implicitly converted, you can already set a project setting to an int and it will convert it to Variant. On getting a setting you can use AsInt32() or cast it to (int) to convert the Variant to the type you want. So a generic method wouldn't be much shorter.

One of the inconveniences of using generic methods in here, which is the main reason that Variant was created to begin with, is that it would allow the user to pass any type (including types not compatible with Variant) which means if the type is not compatible it will fail when passing it to Godot, this means that while the code will compile it will fail on runtime and we want to move as many errors to compile time as possible to ensure safety.

If we were to add those generic methods, we would have to use the [MustBeVariant] attribute on the T to keep the safety guarantees, but at this point I'm not sure this would really add much value over what we already have.

I can see value on having a generic Variant.CreateFrom and Variant.ConvertTo but I would consider those advance unsafe features and I wouldn't want those to be easily accessible to prevent beginner users from accidentally stumbling upon them and using them without knowing what they are doing because it will most likely result in runtime errors which are not very developer friendly.


TL;DR: I don't think we want to go back to using generic types without constraints as Variant, since that's the reason we moved away from System.Object to begin with, see #3837 for more details. But it might be a good idea to provide some way to explicitly convert any T from/to Variant as an advanced feature that trades compile-time safety for runtime errors, it would likely be added to VariantUtils.

@avilches
Copy link
Author

Thanks you for your comment! I guess having a good way to convert T from/to Variant could be interesting in the future. :)

@raulsntos
Copy link
Member

Implemented by godotengine/godot#68310 which adds this API:

partial struct Variant
{
	public static Variant From<[MustBeVariant] T>(in T from);
	public T As<[MustBeVariant] T>();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants