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

TypeDescriptor.AddAttributes() with TypeConverterAttribute not working in content importer #3387

Open
Inverness opened this issue Jan 13, 2015 · 6 comments
Labels
ContentPipeline Content processing issues

Comments

@Inverness
Copy link
Contributor

Something I've done for quite some time in my game is use TypeDescriptor.AddAttributes() to specify type converters for common types like Vector2 and Rectangle so they're serialized and deserialized as a comma-delimited string.

When trying to use this same strategy in a content importer with MGCB, it fails to make any change to the converter returned with TypeDescriptor.GetConverter(). I'm mystified as to what could be causing this behavior.

Example:

TypeDescriptor.AddAttributes(typeof(Rectangle), new TypeConverterAttribute(typeof(RectangleConverter)));
TypeConverter converter = TypeDescriptor.GetConverter(typeof(Rectangle));

The converter variable is the default TypeConverter instead of a RectangleConverter as it should be. At first I thought it was some oddity with JSON.NET but having the following GetConverter() call not return the proper type seems to indicate otherwise.

@tgjones
Copy link
Contributor

tgjones commented Jan 13, 2015

It's probably because of this:

https://github.com/mono/MonoGame/blob/develop/MonoGame.Framework.Content.Pipeline/Graphics/VertexChannelGeneric.cs#L83

MonoGame itself calls TypeDescriptor.AddAttributes, and I'd guess that MonoGame is "winning" when you call TypeDescriptor.GetConverter. (XNA is different - it uses a [TypeConverter] attribute directly on the Vector3 struct declaration, which is probably why you could previously override it using AddAttributes.)

I assume there was a good reason why it's done this way in MonoGame. Without knowing the history, I'm not sure why we couldn't use [TypeConverter] on the platforms that support it, especially since the content pipeline always (?) references the desktop versions (Windows, Mac, Linux) of MonoGame, which do (?) have [TypeConverter] available.

EDIT: actually I can't find anywhere that MonoGame registers a TypeConverter for Rectangle - the code I linked to above is only for vector types. So maybe this is a red herring.

@tomspilman
Copy link
Member

I assume there was a good reason why it's done this way in MonoGame.

I don't remember for certain why, but one potential issue is that WinRT doesn't support TypeConverterAttribute. We could potentially #if it in there and it be ok.

@Inverness
Copy link
Contributor Author

@tgjones As far as I know, calling AddAttributes for attributes already part of a class will replace them rather than be ignored. So even if MonoGame does this my change should have overwritten it for use in my importer.

Also, I'm not migrating from XNA so that really isn't a factor in my case. This is a case of using MonoGame where AddAttributes() works correctly at runtime but not in the content pipeline.

@Inverness
Copy link
Contributor Author

I've done some more looking and managed to find the solution. The issue is due to some quirk about how assemblies are loaded dynamically.

Details here and here

So it's not a problem with MGCB, but it could be a potential problem for anyone else that might try to use their own type converter for content building.

@XtroTheArctic
Copy link

Hello.

I am having the same issue on a Unity project but the solution provided by Inverness doesn't fix it.

Any idea?

Notice: Unity is working on mono environment and I asked the same question in here: http://answers.unity3d.com/questions/1274587/why-does-typedescriptoraddattributes-not-work-in-u.html

@Inverness
Copy link
Contributor Author

@XtroTheArctic I see from the link that this seems to be a bug in how Mono retrieves type converters.

If this is for JSON serialization you'll need to do a workaround by adding your own converters to the JsonSerializerSettings.

@Jjagg Jjagg added the ContentPipeline Content processing issues label Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ContentPipeline Content processing issues
Projects
None yet
Development

No branches or pull requests

5 participants