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

renderer: pass errors via SDL_SetError #1919

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

takase1121
Copy link
Member

@takase1121 takase1121 commented Oct 27, 2024

This is a thing that everyone wants but it keeps delayed for no good reason.
This PR adds error messages by using SDL_GetError(). There are a few ways that we can pass error codes around the renderer:

  1. Use some error code. Unfortunately, SDL doesn't use it, and FreeType error codes are limited (can't open file? why?). There is also the question of which set of error codes to return - FreeType, our own, or what? Or we can do it like Windows, by indicating whether the error is generated by SDL or FreeType with the high byte? Wouldn't that still clash with FreeType?
  2. Return some kind of error type that be casted to a string, directly or indirectly. This still raises questions regarding if that string should be statically allocated, or comes with a complementary free function, etc.
  3. Just use SDL_SetError() and SDL_GetError(). I'm pretty sure this way of error handling is shunned by a lot of people, but face it - this is just the most flexible way when you have error messages from various sources, and in various formats. SDL_GetError() also stores the message in thread local storage, so no problems with multithreading either.

Obviously, this PR uses approach 3. Finally we can get stuff like this:
image

Obviously that message is kinda long, but this beats not having a proper error message AT ALL.

As a side note, this also implicitly implies that we should adopt a 0 for success and negative for failure convention, as used by SDL. I don't think this is much problem, since we partially use 0 for success already in various places in the codebase.

Closes #1721.

Copy link
Member

@Guldoman Guldoman left a comment

Choose a reason for hiding this comment

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

I guess this is the simplest way to pass errors around.
My only problem is possibly forgetting to use SDL_ClearError when we add this to more places.

@Guldoman
Copy link
Member

Guldoman commented Dec 4, 2024

Closes #1721.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better errors when loading a font fails
2 participants