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

Adds a description to an InvalidOperationException for clarity. #8019

Conversation

rbwhitaker
Copy link
Contributor

This should "fix" #8004, if that's something the maintainers wish to do something about.

I was porting some really old XNA code to MonoGame and bumped into an issue where I got an InvalidOperationException. I think the exception was correct, but in order to understand what was going on, I had to pull down the code and step through it to discover that the effect's type was expected to implement IEffectMatrices. Once I saw that, I knew how to fix it. But I had to dig quite deep to find it.

This change would hopefully prevent somebody else from having to do the same digging in the future.

On the original issue, there was a comment by @nkast suggesting another possible solution: Just do a cast and let the runtime handle things. I feel that solution would also address my problem, if we'd rather go that route. Though that might run a slightly higher risk of breaking people, since it changes the exception type.

If the maintainers would prefer that solution, or any other tweaks to this solution, let me know and I'll change it.

If we don't want to do anything with this at all, you won't hurt my feelings to just decline this PR.

@rbwhitaker
Copy link
Contributor Author

I updated the text shown in this exception to match what XNA does, as shown in the screenshot here: #8004 (comment)

@harry-cpp
Copy link
Member

Non descriptive error messages are the worst :D Thanks! Merging!

@harry-cpp harry-cpp merged commit bcde6e1 into MonoGame:develop Apr 18, 2023
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants