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

added shader compilation check for ImmediateModeRenderer20 #6321

Merged
merged 2 commits into from
Dec 19, 2020

Conversation

mgsx-dev
Copy link
Contributor

@mgsx-dev mgsx-dev commented Dec 19, 2020

It might sounds like a breaking change but i can't see a case where user code check compilation and fallback to something else so i think it's fine to merge it as is.

@mgsx-dev mgsx-dev added core affecting all platforms enhancement labels Dec 19, 2020
@crykn
Copy link
Member

crykn commented Dec 19, 2020

Is an IllegalArgumentException the right type of exception in this case? Because I think the compilation only fails if there are some weird platform specific issues -> Maybe a GdxRuntimeException would fit better. But anyway, looks good to me.

@MobiDevelop
Copy link
Member

Yeah, I'd agree - IllegalArgumentException implies that the user supplied something invalid, so if that's not really the case here, it'd be better to use a different exception.

@crykn
Copy link
Member

crykn commented Dec 19, 2020

SpriteBatch seems to be throwing an IllegalArgumentException as well, so I think it should be changed there, too.

@mgsx-dev
Copy link
Contributor Author

I simply copied what we have at other places (eg. SpriteBatch, CameraGroupStrategy) to be consistent but i agree with you, that was stupid :D. I'm not sure it worth changing exception type at other places since it'd be a not so valuable breaking change. Anyway it's beyond the scope of this PR.

Copy link
Member

@tommyettinger tommyettinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks absolutely fine. An extra check for correctness is nice.

@NathanSweet NathanSweet merged commit 3c9a574 into libgdx:master Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core affecting all platforms enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants