Improved Surface.fill error message for invalid blend flags

[Matiiss]

Test code:

import pygame

image = pygame.Surface((5, 5))
try:
    image.fill((255, 0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)
except pygame.error as e:
    print(f"{e = }")
else:
    print("no error")

image = pygame.Surface((5, 5), pygame.SRCALPHA)
try:
    image.fill((255, 0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)
except pygame.error as e:
    print(f"{e = }")
else:
    print("no error")

Currently it either simply raises a pygame.error without any error message or I have managed to get Surface doesn't have a colorkey in this case:

import pygame

pygame.display.set_mode(flags=pygame.HIDDEN)
image = pygame.image.load("scenery.jpg").convert()
try:
    image.fill((255, 0, 0), special_flags=pygame.BLEND_PREMULTIPLIED)
except pygame.error as e:
    print(f"{e = }")
else:
    print("no error")

[Matiiss]

Is code robustness an appropriate label in this case?

[Matiiss]

Maybe it should say "given blend flag is not supported for this operation" instead?

[ankith26]

LGTM, thanks for the PR!

Some other notes up for discussion, but IMO none blockers for this PR

• Maybe a small unit test would be nice
• Should the python error returned be a ValueError?

[Matiiss]

@ankith26
About the ValueError, as I see it, that would require returning some other value as the result, for example, -2 and then adding an else if after this block

pygame-ce/src_c/surface.c

Lines 1857 to 1858 in 0168adc

	if (result == -1)
	return RAISE(pgExc_SDLError, SDL_GetError());

to check for that value and raise a separate error. But that would make more sense I guess.

I will add a test though, since currently I couldn't find any that would test for any errors raised by Surface.fill.

[zoldalma999]

LGTM!

[Starbuck5]

What if you set the error message with SDL_SetError and then have it handled by the existing SDL error handler?

[Matiiss]

What if you set the error message with SDL_SetError and then have it handled by the existing SDL error handler?

That is what the first commit did, however, Ankith suggested raising a ValueError and I thought it would be more suitable too, so changed to that.

[Starbuck5]

I think this should be an SDL error, since it should look like it's coming from an SDL fill system. Just like how in alphablit.c we implement a blitter and use SDL_SetError there.

[ankith26]

I'm fine with pygame.error