-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
Font constructor now raises an error when it fails to load a valid font file #3064
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this PR I get a pygame.error: Passed a NULL pointer
error, after this PR I get pygame.error: Couldn't load font file
which is much better OFC
Thanks! 💯
# for that version of SDL_ttf) | ||
font_path = os.path.join("examples", "data", "MODERN.fnt") | ||
self.assertRaises( | ||
(pygame.error, FileNotFoundError), lambda: pygame_font.Font(font_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could this raise a FileNotFoundError?
pygame_font here could be ftfont based on freetype?
freetype.font segfaults on my system when I try to load this path, but maybe on some other systems it raises filenotfound.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I tracked down to where the segfault happens. Didn’t figure out exactly what went wrong, but the function call is here:
https://github.com/pygame-community/pygame-ce/blob/main/src_c/freetype/ft_wrap.c#L343
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it doesn’t segfault, freetype raises filenotfound
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So pygame_font is either Font or ftfont, and this PR introduces a difference between the Font and ftfont APIs (which are supposed to match).
This is why I bring it up. The test shouldn't need to test for either behavior, it should test for the one correct behavior which should be shared.
Sample piece of code:
Sample font file (same file is added in the examples dir in this pull):
MODERN.zip
Source for the sample file