| Summary: | SDL_GetColorKey doesn't set error message | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Luke A. Guest <laguest> |
| Component: | video | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | enhancement | ||
| Priority: | P2 | CC: | sezeroz, sylvain.becker |
| Version: | 2.0.7 | ||
| Hardware: | x86_64 | ||
| OS: | Linux | ||
|
Description
Luke A. Guest
2017-12-05 15:13:35 UTC
Fixed, thanks! https://hg.libsdl.org/SDL/rev/cf9c6a79123f I don't think the second "SetError()" is a good idea:
"return SDL_SetError("Surface doesn't have a colorkey");"
Because the purpose and usage of SDL_GetColorKey() is also to tell whether the surface has a colorkey or not. And it will report false errors.
I disagree, you need the error to distinguish between a NULL surface error and a surface that doesn't have a colorkey. > you need the error to distinguish between a NULL surface
> error and a surface that doesn't have a colorkey.
The NULL surface error is already identifying itself by the
newly added SDL_InvalidParamError("surface"), but why does
a surface w/o a colorkey need to set an error?
Yes, I mean't this and also: it simply reports a non-error as an error, which is confusing. And it's bothering, if this non-error message replaces a real previous error message. If you previously called another API without a surface you'd think that you passed a NULL surface to this function, so an explicit state needs to be set if you're checking the error string for failure condition. Really the API should be changed to return 1 if the colorkey is set, 0 if not, and -1 on error. We can make that change for SDL 2.1. (In reply to Sam Lantinga from comment #6) > If you previously called another API without a surface you'd think that you > passed a NULL surface to this function, so an explicit state needs to be set > if you're checking the error string for failure condition. yes you're right, got it! |