-
-
Notifications
You must be signed in to change notification settings - Fork 164
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 a mixer.Channel.id
getter
#2369
Conversation
@oddbookworm circleci is failing for you but it's passing on main. After scratching my head for a second on this, I believe it's because it's pulling the circleci config from YOUR (outdated) main branch, rather than the PR branch or pygame-ce's main branch. @ankith26 tagging for visibility. I haven't thought about mitigation yet, but presumably we'd want circleci to use the config of the PR branches themselves, is there a way to have that? |
Alright, I pulled in the most recent merges to both my own |
Yes I'd like that too, but unfortunately there's nothing much we can do about it AFAIK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
It's not earth shattering as users could already keep track of their own channel IDs mapping to channels if they wanted, but might make it slightly easier to find out/debug which channel a sound is playing on with a simple number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is new API, how about considering making this a read-only property instead? Fewer characters to type and also pythonic
I'll push a commit soon for this |
38f8a5a
to
0c3e2a5
Compare
87ce498
to
b445617
Compare
force push to fix a half-complete commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the PR 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
No description provided.