Skip to content
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

repr() of some classes doesn't use subclass name #3044

Open
aatle opened this issue Aug 7, 2024 · 4 comments
Open

repr() of some classes doesn't use subclass name #3044

aatle opened this issue Aug 7, 2024 · 4 comments
Labels
bug Not working as intended

Comments

@aatle
Copy link
Contributor

aatle commented Aug 7, 2024

Environment:

pygame.print_debug_info()
pygame-ce 2.5.0 (SDL 2.30.3, Python 3.12.4)
Platform:               Windows-11-10.0.22631-SP0
System:                 Windows
System Version:         10.0.22631
Processor:              Intel64 Family 6 Model 167 Stepping 1, GenuineIntel
Architecture:           Bits: 64bit     Linkage: WindowsPE

Python:                 CPython 3.12.4 (tags/v3.12.4:8e8a4ba, Jun  6 2024, 19:30:16) [MSC v.1940 64 bit (AMD64)]
pygame version:         2.5.0
SDL versions:           Linked: 2.30.3  Compiled: 2.30.3
SDL Mixer versions:     Linked: 2.8.0   Compiled: 2.8.0
SDL Font versions:      Linked: 2.22.0  Compiled: 2.22.0
SDL Image versions:     Linked: 2.8.2   Compiled: 2.8.2
Freetype versions:      Linked: 2.11.1  Compiled: 2.11.1

Display Driver:         Display Not Initialized
Mixer Driver:           Mixer Not Initialized

Current behavior:

In some classes such as Surface, Rect, Vector2, and others, the repr() behavior uses a hard-coded class name string instead of a dynamic access of the class name.
Subclass instances and normal instances appear the same with repr().
Example with Vector2 subclass:

>>> repr(MyPosition(0.1, 0.2))
'Vector2(0.1, 0.2)'

Expected behavior:

The repr() should include the name of the actual instance class instead of the class that implemented the functionality.

>>> repr(MyPosition(0.1, 0.2))
'MyPosition(0.1, 0.2)'

Test code

import pygame as pg

class S(pg.Surface):
  __slots__ = ()

print(S([1,1]))

class R(pg.Rect):
  __slots__ = ()

print(R(0,0,5,5))

class V(pg.Vector2):
  __slots__ = ()

print(repr(V(1.2, 1.3)))

Side note:
I think the Surface repr should also say when it has per-pixel alpha.

@aatle aatle added the bug Not working as intended label Aug 7, 2024
@oddbookworm
Copy link
Member

I'm not sure that this is something we should do. Most pygame-ce objects, while they can be subclassed, don't make much sense to do so regularly. IMO if you're subclassing these classes, then you should be writing your own custom __repr__ anyway because the assumption there is that you've made a significant enough change on top of the base class that you should be outputting that information anyway. As for the per-pixel alpha in the Surface repr, it's probably possible, but I don't immediately know how best to do it. We might be able to use SDL_GetSurfaceBlendMode and guess that the surface has per-pixel alpha if it's SDL_BLENDMODE_NONE, but I don't know that it's 100% reliable. We could also check the number of bytes per pixel, but again, I'm not sure that's reliable.

@Starbuck5 might have more insight into this than I do though

@aatle
Copy link
Contributor Author

aatle commented Aug 9, 2024

For the Surface repr, I will also note that all per-pixel alpha surfaces show global_alpha, e.g. <Surface(640x360x32, global_alpha=255)>, which seems a bit strange. Moreover, documentation states that per-pixel alpha is incompatible with colorkeys and global surface alphas. Though, from my understanding, the docs may be heavily outdated about surface transparency (possibly needs another issue).

For class __repr__, I have a few points.
The repr() should always have the name of the exact type of the object (for a few different reason not mentioned here). Getting the name dynamically guarantees this, even if the class name is changed or the class is subclassed. Which is why it is common in python to do this in __repr__ implementations.
IMO, the fact that a class isn't usually subclassed, or should get a new __repr__ anyway, aren't really reasons why hard-coded class names are better. They are more like reasons why implementing this change is not that useful. However, I don't see any immediate downsides, and in my use case it would be nice to have.
(Though, it's not that important, and there's a ton of inconsistency anyway within pygame and even the python standard library, regarding this.)

@oddbookworm
Copy link
Member

The reason global alpha isn't independent of per-pixel alpha is that they can be used together
image

@aatle
Copy link
Contributor Author

aatle commented Aug 9, 2024

Hmm, I guess a lot of the other pygame.Surface docs regarding transparency need updating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended
Projects
None yet
Development

No branches or pull requests

2 participants