pygame icon indicating copy to clipboard operation
pygame copied to clipboard

Unify version getters

Open Starbuck5 opened this issue 3 years ago • 5 comments

Pygame depends on several C libraries, and occasionally people might want to know the versions of said libraries, so pygame provides version getter functions.

SDL: pygame.get_sdl_version() SDL_ttf: missing Freetype: pygame.freetype.get_version() SDL_image: pygame.image.get_sdl_image_version() SDL_mixer: pygame.mixer.get_sdl_mixer_version()

These all have slightly different semantics, it would be a cool cleanup effort to get them going the same way, and to add an SDL_ttf version function.

pygame.get_sdl_version returns what SDL was compiled against pygame.freetype.get_version returns what freetype was compiled against pygame.mixer.get_sdl_mixer_version returns the runtime version by default, but can also return the compiled version with a parameter pygame.image.get_sdl_image_version returns what SDL_image was compiled against, or None

My feeling is that they should all work like get_sdl_mixer_version. They should all default to the runtime check, with the compile time check as a backup. This is a behavior change but I don't think this would introduce backwards compatibility problems.

Tasks:

  • [ ] convert existing version functions to work like get_sdl_mixer_version
  • [x] add a new version function for SDL_ttf

Starbuck5 avatar Mar 20 '22 06:03 Starbuck5

I just noticed this code in get_sdl_mixer_version that may need to be fixed up

    if (NULL != linkedobj) {
        linked = PyObject_IsTrue(linkedobj);

        if (-1 == linked) {
            return RAISE(PyExc_TypeError, "linked argument must be a boolean");
        }
    }

It looks like the author intended this to be a true/false check, with a fail if not a boolean. But it's really a truthiness check. You could pass in the string "pygame" and the function would interpret that as linked = True.

Starbuck5 avatar Mar 20 '22 07:03 Starbuck5

+1 for the general idea, I would love to see more consistency here, and keyword argument for users to get access to both compiled and linked versions sounds useful for reporting bugs related to the respective submodules.

However I believe that the compiled version is often the more useful number for users experimenting with things, so that should be returned by default, this is also inline with most of the existing functions behaviour.

About get_sdl_mixer_version, we can either

  1. keep things as is, return linked version by default
  2. break compatibility a bit, return compiled version by default, make linked=False the default

I don't have a particular preference in the above 2 options, anything should be fine really

ankith26 avatar Mar 20 '22 10:03 ankith26

However I believe that the compiled version is often the more useful number for users experimenting with things, so that should be returned by default, this is also inline with most of the existing functions behaviour.

I disagree, I think the actual library version running seems more helpful.

get_sdl_mixer_version is the only one that does it now, but is also the most modern of the functions. I tracked down the PR it's from: https://github.com/pygame/pygame/pull/1196

Starbuck5 avatar Mar 26 '22 09:03 Starbuck5

Some extra questions:

  • What should be the future of pygame.version module?
  • Should we also include those functions there?
  • How are we gonna treat SoftwareVersion class that is used by pygame.version.SDL compared to other functions that use tuple of ints?

MightyJosip avatar Mar 28 '22 19:03 MightyJosip

So, I think we can standardize the bulk of the version functions without touching pygame.version. However, it is a good question.

I do like having the dependencies tied to the module where they are used. So I don't see a need to export all that information to the pygame.version module.

PR adding pygame.version.SDL: https://github.com/pygame/pygame/pull/1710. It was a feature request by robertpfieffer but it doesn't seem to have had any discussion, at least on Github.

One thing we could do @MightyJosip if you really want a standardization is make them all return a namedtuple [major, minor, patch]. Or I suppose the existing SoftwareVersion class would also work.

>>> SoftwareVersion = namedtuple("SoftwareVersion", ["major", "minor", "patch"])
>>> a = SoftwareVersion(2,0,20)
>>> a
SoftwareVersion(major=2, minor=0, patch=20)

Starbuck5 avatar Mar 28 '22 23:03 Starbuck5