BlobArray with ValueType Generic Parameters fails

As of Entities 0.17.0-preview.41 I cannot have a BlobArray with a value type containing generic parameters that are constrained to be value type.

Here is an example of data struct:

Code (CSharp):

1.     public struct MyData<T> where T : struct {
2.       public T Value;
3.     }

Here is my method to create the Blob Asset Reference:

Code (CSharp):

1.     public static BlobAssetReference<BlobArray<MyData<TData>>> CreateClipBlobReference<TData>(NativeArray<TData> sourceData)
2.       where TData : struct {
3.  
4.       using (var builder = new BlobBuilder(Allocator.Temp)) {
5.         ref var root = ref builder.ConstructRoot<BlobArray<MyData<TData>>>();
6.         var clips = builder.Allocate(ref root, sourceData.Length);
7.         for (var i = 0; i < sourceData.Length; ++i)
8.           clips[i].Value = sourceData[i];
9.         return builder.CreateBlobAssetReference<BlobArray<MyData<TData>>>(Allocator.Persistent);
10.       }
11.     }

The error is misleading since there is no reference or pointer type in the data:
error ConstructBlobWithRefTypeViolation: You may not build a type BlobArray`1 with Construct as BlobArray`1[].Value is a reference or pointer. Only non-reference types are allowed in Blobs.

The problem lies in the IsValueType() Cecil extension for TypeReference @ Unity.Entities.CodeGen\CecilExtensionMethods.cs

An easy fix would be to check if any constraint of a generic type is of "System.ValueType" instead of assuming that all generic types are not a value type.

Code (CSharp):

1.       if (typeReference is GenericParameter &&
2.         (typeReference as GenericParameter).Constraints.Any(p => p.FullName == "System.ValueType"))
3.         return true;

Spoiler: Full IsValueType() extension

public static bool IsValueType(this TypeReference typeReference) {
if (typeReference is GenericParameter &&
(typeReference as GenericParameter).Constraints.Any(p => p.FullName == "System.ValueType"))
return true;

if (typeReference is ArrayType)
return false;

if (typeReference is PointerType)
return false;

if (typeReference is ByReferenceType)
return false;

if (typeReference is GenericParameter)
return false;

var pinnedType = typeReference as PinnedType;
if (pinnedType != null)
return pinnedType.ElementType.IsValueType();

var requiredModifierType = typeReference as RequiredModifierType;
if (requiredModifierType != null)
return requiredModifierType.ElementType.IsValueType();

var optionalModifierType = typeReference as OptionalModifierType;
if (optionalModifierType != null)
return optionalModifierType.ElementType.IsValueType();

var typeDefinition = typeReference.Resolve();

if (typeDefinition == null)
throw new InvalidOperationException($"Unable to locate the definition for {typeReference.FullName}. Is this assembly compiled against an older version of one of its dependencies?");

return typeDefinition.IsValueType;
}

It would be nice to have this fixed because i use this in some of my generic systems, and right now i have to edit the Entites source code package to have this working as mentioned above

 

Unity already aware of that, for now solution is package source fix, until they implement proper fix.
https://forum.unity.com/threads/entities-0-17-changelog.1020202/page-2#post-6759856

 

Sorry to bring that back up but it still happens in 0.50.
Is that an overseight or is it intened not to allow this ??

 

Ok. I see it's a bit more involved than I thought.
Thanks for the explanation.
I'll try to think about a work around for may use case because for now having a local copy of the entities package that check the value type works but it is a pain in the .

 

If we consider that is an advanced feature, would it be possible to forbid generic type except if they implement the interface.

Code (CSharp):

1.             if (typeReference is GenericParameter &&
2. if (typeReference is GenericParameter && typeof(IBlobType).IsAssignableFrom(Type.GetType(typeReference.FullName + ", " + typeReference.Module.Assembly.FullName)))
3.             {
4. )
5.             {
6. return true;
7. }
8. if (typeReference is GenericParameter)
9.             {
10. return false;
11. }
12.  
13.  

The IBlobType documentation could specify what it is and what it should ensure.
You could even put the type under an unsafe namespace to "scare off" less experienced users and emphasize the fact that is may mess things up.
And have another compiler validator that check that all IBlobType are safe ?

Or worst case scenario hide that possibility behind a scripting define like DISABLE_GENERIC_BLOB_VALIDATOR or something...

I get that you want to make sure things are safe to use and that the vast majority of people would certainly mess things up. But I think it's a shame to forbid some features in the name of safety. With that philosophy there should be no unsafe container available to users...

 

IMO, just constrain on `unmanaged` type would be enough for blob.
For those who need pointer in blob (or any context) they must use `unsafe` keyword, which is an announcement of "I know it is **unsafe** here, but I really need to use pointer, and its on me if its break the system".

And even if you restrict pointer out from blob, you can still use some magic, well, just use `long`, to workaround those restrictions.

Any other formatter/serializer won't restrict user from serialize pointer to its own format, why would blob do?

 

Can you confirm that NativeLists<T> are broken now as well? The following worked just fine prior to entities 0.50

 

Hmm...looks like changing

Code (CSharp):

1. where T : struct

to

Code (CSharp):

1. where T : unmanaged

fixes this:

 

@elliotc-unity Any news on the blob asset + generics front? It's becoming rather costly to have to manually disable this, IMO very over-zealous, safety check every time I update the Entities package.

Either solution is fine by me: add a DISABLE_GENERIC_BLOB_VALIDATOR or an interface to tag possible types that'll go into it. Since the latter is kind of a breaking change and you just released 1.0 without it, I imagine that is out of the conversation. But, the scripting define is a low effort solve for this and opt-in.

On a side note, I still don't understand why BlobAssets are in Entities and not in Collections btw... it's very weird that the go-to way to access read-only data in bursted jobs can't be used unless you drag all of entities into your project. Seems to go against the "DOTS is a stack. ECS is just a part of it." motto that got thrown around a lot.