Add a description on an InvalidOperationException thrown from Model.Draw(Matrix, Matrix, Matrix)

[rbwhitaker]

I was playing with custom effects, set up my own effect (from the template), loaded it, and then attached it to a model, then ran it. My code already had a line like model.Draw(world, view, projection), and this line threw an InvalidOperationException. That's because my effect did not implement IEffectMatrices, which is a decent reason to fail. My problem is that the error message gave me little to go on. I had to dig into the source code to find the issue.

What if we changed that line in Model.Draw(Matrix world, Matrix view, Matrix projection) to be something more like throw new InvalidOperationException("effect must implement IEffectMatrices to support this call"); or something?

That at least points people in a reasonable direction if they do what I did, rather than being faced with a completely opaque error. (I had initially assumed something had been disposed.)

I'd be willing to make a PR for it, as long as I knew it was something people were interested in.

What version of MonoGame does the bug occur on:

• MonoGame 3.8.1

What operating system are you using:

• Windows

What MonoGame platform are you using:

• DesktopGL

[Ravendarke8]

I get where you are coming from, however I am not sure if XNA model is worth further support. If mg team would have maintenance capacity to spare (even to simply integrate your PR), I would say sure, 100%, go for it. Given current state of things (and state of XNA model to begin with, which is not MG's team fault in any way), it might be difficulty to dedicate any resource to it (even tho objectively it would require fairly little).

To be honesty, I've wrote quite a few custom effects and model systems back in the day, and I don't remembering using IEffectMatrices once.

Anyway, I have no horse in this race, if you make PR that gets integrated, great, just my two cents regarding Model/Effect

[nkast]

That would be helpful. Here's how I changed the code.

I stayed out of the way and let the CLR do it's job.
In case the cast fail, the system will throws an InvalidCastException.

                {
	   	    IEffectMatrices effectMatricies = (IEffectMatrices)effect;
                    effectMatricies.World = sharedDrawBoneMatrices[mesh.ParentBone.Index] * world;
                    effectMatricies.View = view;
                    effectMatricies.Projection = projection;
                }

[rbwhitaker]

I'm not opposed to that solution, and kind of like it. It does change the API, technically. What once threw an InvalidOperationException would now throw an InvalidCastException, and it is possible (but I'd think highly unlikely) that somebody is catching the current InvalidOperationException and handling it otherwise. The change in exception type could break that. Strictly speaking, even changing the text, as I initially suggested, could break something if people are keying off the message to do stuff.

I'm not sure I've got a good feel for how tolerant MonoGame is for API changes. I know MonoGame has generally been a stickler for XNA API compatibility, but looking at the docs for XNA I don't see any documentation about the Draw method even throwing an exception of any type. So I'm not sure the exception type is even considered part of the public API. Perhaps it can be changed without any API concerns.

This is one of those tiny things where I'd happily put up a PR for it, if I knew what direction the maintainers preferred. But perhaps this is small enough I could just put up a PR and just see what happens with it. Keep the paperwork minimal and just let it go pass/fail instead of needing a reply before trying it. :D

[nkast]

In your PR you said that the error was caused after porting an old XNA code.
That code worked in Xna even though you hadn't implemented IEffectMatrices?

https://shawnhargreaves.com/blog/effect-interfaces-in-xna-game-studio-4-0.html

[nkast]

Well, you are right. MonoGame is on par with Xna on this. I've tested it with the CustomModelEffect Sample.
Also the InnerException of InvalidOperationException is null.

[rbwhitaker]

In your PR you said that the error was caused after porting an old XNA code. That code worked in Xna even though you hadn't implemented IEffectMatrices?

Er... it was based on an XNA sample, but I had made some tweaks that made mine crash. I don't remember the details anymore, but the exception makes sense in XNA Land, for what I was doing, as well.

[rbwhitaker]

Thanks, @nkast, for sharing that screenshot. I'm going to update the PR to use that text. I like how it is phrased. Descriptive and gives an alternative.