Skip to content

Remove surflock as the module #3417

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MightyJosip
Copy link
Member

Atm, surflock is empty module, without any function, also without documentation. However, it is needed for Surface. This PR removes the module, and moves the functionality to the surface.

Python users shouldn't notice any changes (nor they were ever supposed to use that module)

However, if someone has been using pygame C API, old functions have been moved from surflock to surface module, so they would need to update their code

@MightyJosip MightyJosip requested a review from a team as a code owner April 28, 2025 09:49
@damusss damusss added Code quality/robustness Code quality and resilience to changes Surface pygame.Surface labels Apr 28, 2025
Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not usually a good idea to remove something, but this module has always literally been empty, I find it absolutely impossible for this change to break anyone's code. I am a fan of removing obsolete pygame submodules so I'm going to approve it.

@Starbuck5
Copy link
Member

Why not leave it as a separate file but use the functions in surface.c?

I think the original intention of this file was to split up some related surface functionality. Not have files so long.

I find it absolutely impossible for this change to break anyone's code
obsolete pygame submodules

Yes this could break someone's code, no it's not an obsolete submodule just because it doesn't export functions directly to the user. pygame.base doesn't export functions to the user either. Surflock has a C API and if anyone's using it, this will absolutely break their code. I know we don't typically care about C API backwards compat but the C API is publicly documented.

@Starbuck5
Copy link
Member

Starbuck5 commented Apr 29, 2025

Last year I spent quite a while trying to merge pygame-ce submodules, rwobject was my early target. I was hopeful it could increase the compiler ability to optimize, make our codebase more typical for a C codebase, and I got promising results that a smaller number of binaries would be meaningfully more space efficient. I couldn't get a solution good enough to incrementally port some modules using the classic C API and some modules not using it.

Anyways I support this effort in general but our files are already so long, and the surflock stuff is so nicely separated into a different unit of abstraction, I would like to keep it a separate file even if it's not technically a Python module.

@MightyJosip
Copy link
Member Author

Valid points. My main goal for this was to remove this part https://github.com/pygame-community/pygame-ce/pull/3417/files#diff-41261be2b7f66f350124346cb97d5e6af46016b07c7d82d4fa9120b359def0bfL208-L243, since it has no use for the python side. Anyways, I guess I will wait a bit and reconsider this, coz I have really big refactor changes in progress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality/robustness Code quality and resilience to changes Surface pygame.Surface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants