search

libsdl-org / sdl12-compat

An SDL-1.2 compatibility layer that uses SDL 2.0 behind the scenes.
Other
191 stars 40 forks source link

SDL_SetPalette incorrect return value? #294

Closed tylerlm closed 1 year ago

tylerlm commented 1 year ago

In SDL 1.2, SDL_SetPalette returns 1 on success ("if all colors were set as requested") or 0 otherwise: SDL_video.c

In sdl12-compat, SDL_SetPalette returns 0 on success, or -1 or SDL20_OutOfMemory() on failure: SDL12_compat.c

(Other issues that mention the call are #216 and #283, though I'm not sure they're related)

tylerlm commented 1 year ago

Thanks!

I think the no-ops on lines 7306,7310,7315 should also be 1.

icculus commented 1 year ago

No, they return 0 in classic SDL 1.2, also:

https://github.com/libsdl-org/SDL-1.2/blob/main/src/video/SDL_video.c#L1280-L1282

https://github.com/libsdl-org/SDL-1.2/blob/main/src/video/SDL_video.c#L1291-L1295

tylerlm commented 1 year ago

Oops, you're correct, my mistake!

@icculus, @slouken thank you for catching that, sorry for the breakage!

tylerlm commented 1 year ago

Ahem, my merge needs to be reverted; is this appropriate?

I thought "surely these three integers will be trivial enough for a baby PR, what could go wrong". Ah well.

icculus commented 1 year ago

I thought "surely these three integers will be trivial enough for a baby PR, what could go wrong". Ah well.

No worries, thank you for taking interest in the tiny details!