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

Add pygame 2 #247

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add pygame 2 #247

wants to merge 3 commits into from

Conversation

sparshg
Copy link

@sparshg sparshg commented Jun 4, 2023

Since pygame 2 is based on SDL2, its dependencies are also added.

@sparshg sparshg requested review from orowith2os and a team as code owners June 4, 2023 15:43
@A6GibKm
Copy link
Collaborator

A6GibKm commented Jun 4, 2023

Could you please give some examples of apps that make use of this module as bundled here?

@sparshg
Copy link
Author

sparshg commented Jun 4, 2023

Currently we are updating the flatpak packages like Physics and Pippy which require this module. We are also planning to add more packages which may use pygame.

SDL2/SDL2_ttf-2.20.2.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@hfiguiere hfiguiere left a comment

Choose a reason for hiding this comment

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

Don't add unecessary modules

pygame2/pygame-2.4.0.json Outdated Show resolved Hide resolved
@chimosky
Copy link

chimosky commented Jun 5, 2023

Don't add unecessary modules

It isn't clear which modules are unnecessary as same modules were bundled with SDL, looking around does help for sure but that won't be clear to someone contributing for the first time.

@Erick555
Copy link
Collaborator

Erick555 commented Jun 5, 2023

The person who adds shared-module is expected to maintain it for the foreseeable future. They should know the build details and what dependency is necessary.

@chimosky
Copy link

chimosky commented Jun 5, 2023

The person who adds shared-module is expected to maintain it for the foreseeable future. They should know the build details and what dependency is necessary.

Agreed.

Copy link
Contributor

@orowith2os orowith2os left a comment

Choose a reason for hiding this comment

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

I'm going to have to go with denying this, tbh. It feels better to have in a BaseApp than a submodule, imo.

@barthalion
Copy link
Member

If it takes less than 30 minutes to build, maintenance overhead of a BaseApp is not worth it.

@orowith2os
Copy link
Contributor

If it takes less than 30 minutes to build, maintenance overhead of a BaseApp is not worth it.

I was thinking a more generic BaseApp, that just includes some common game libraries (pygame, newer SDL2, wine, gamemode). This would also solve #221.

@A6GibKm
Copy link
Collaborator

A6GibKm commented Jun 7, 2023

First you need to define a clear usecase, are these apps using wine for example? We already have a base app with wine.

@sparshg sparshg requested a review from hfiguiere June 14, 2023 08:16
],
"buildsystem": "simple",
"build-commands": [
"pip3 install --ignore-installed --no-deps --prefix=/app ."
Copy link
Contributor

@bbhtt bbhtt Sep 10, 2023

Choose a reason for hiding this comment

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

You should run setup.py cython, to recythonise the sources before installing. Upstream c sources might not match with the python C api available in the runtime or app. The c api time to time introduces changes leading to build failures like #201 and the python version is a moving target as it is in shared-modules and will get included in manifests that have different python versions.

https://www.pygame.org/wiki/GettingStarted#Compilation

Some of the .c files are generated by Cython from .pyx files. Running "setup.py cython" will update them.

@hfiguiere
Copy link
Collaborator

You are missing the addition to CODEOWNERS to commit to maintain it.

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

Successfully merging this pull request may close these issues.

8 participants