Deprecating old messy scrap functions and implementing shiny, new SDL-based scrap function#3510
Deprecating old messy scrap functions and implementing shiny, new SDL-based scrap function#3510oddbookworm wants to merge 19 commits into
Conversation
|
All the functions in scrap.c call into functions defined in the old platform specific ones, so the deprecation warnings should only need to be put in scrap.c. Mac seems to always use scrap_sdl2.c, so that is true for it as well, I believe, despite the ifdefs. |
|
@Starbuck5 I addressed your comments and made the necessary corrections |
…tal second deprecation warning in _scrap_get_scrap
|
Looks like you need the changes from #3529 You could rebase your branch or merge main back into this branch. |
MyreMylar
left a comment
There was a problem hiding this comment.
This seems like a good clean up to me. Code looks good and it will work on all SDL platforms as opposed to the patchy, platform specific support from the old days. I will probably transition to using this in Pygame GUI in the future.
I believe the old scrap functions haven't been working correctly for a while now? Meanwhile looking at the SDL clipboard header it has just recently got a new function so perhaps it will be the future of clipboard interaction for pygame.
I'll mark it for a good testing this weekend before final approval, as I want to check the old scrap functionality versus the new on Windows.
| if (!text) { | ||
| const char *err = SDL_GetError(); | ||
| if (err) { | ||
| SDL_free(text); | ||
| PyErr_SetString(PyExc_RuntimeError, err); | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
Hmmm-
so SDL_GetClipboardText says it sends back an empty string on failure, so is that the same as returning NULL? I don't think so? The char* would be a valid pointer to a 1 long array just containing NULL?
And then I have the same question about the point of if (err) like before.
Why is it a runtime error instead of a pygame.SDLerror?
…empty string returned from SDL_GetClipboardText
MyreMylar
left a comment
There was a problem hiding this comment.
OK, I've given the old scrap and this new scrap a good tinker with:
OLD SCRAP
- It does work on Windows as described.
- How it works is a bit python 2 era. E.g. text data is copied and returned as bytes rather than python strings. This is just annoying and messy.
- You can apparently copy and paste wav data but only on windows - but I'm not quite sure how you would do this?
- The main useful thing we are losing on windows from old scrap is the ability to copy directly from MS paint (CTRL +C with an image open in paint) and paste directly into pygame and then use it immediately (after a quick
pygame.image.load(paste_here, ".bmp")) as a Surface. - None of this non-text stuff works on macs.
- The bmp pasting may also work on X11, but probably doesn't work on wayland from what I can see.
- The initialisation of scrap is weird as heck because you have to do it after making a window.
- There is also lots of busy work functions in the old API, as well as the initialisation, multiple ways to find the type of data in the clipboard and some x11 only stuff - I expect to handle the extra middle mouse 'primary selection' clipboard that exists only on Linux.
NEW SCRAP
- Simple three function API. Same on all platforms.
- Returns python strings which is what you want.
- No support for bmp data.
As far as I can see the new scrap is better in all ways except the ability to paste in bmp data on windows (and probably X11) which is probably occasionally useful.
On balance I think we should make this change as it is leveraging SDLs improved text clipboard support - and then investigate just supporting bmp/image data pasting in pygame as sadly this is not in SDL clipboard.
Closes #3402. I added deprecations to all of the old, messy scrap stuff and implemented newer scrap functions based on what SDL2 currently has available (https://wiki.libsdl.org/CategoryClipboard). I know tests still have to be written, but I would like that to be one of the last things I do, so I'm looking for feedback.