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

Deafen and Undeafen members #1272

Merged
merged 4 commits into from
Jan 17, 2025
Merged

Deafen and Undeafen members #1272

merged 4 commits into from
Jan 17, 2025

Conversation

valzargaming
Copy link
Member

Adds deafenMember and undeafenMember functionality to voice channels.

@mikield
Copy link
Contributor

mikield commented Jan 17, 2025

One interesting though that came to me in this PR:

Can we have a context-full resources?
What I mean that, it would be awesome to have something like $channel->members->deafen(ID) instead of calling $channel->deafenMember().

We can return another type of MembersRepostory for voice channels.
So VoiceChannel::memebrs can be VoiceMembersRepository where we can perform voice-specific actions on members.

@valzargaming
Copy link
Member Author

valzargaming commented Jan 17, 2025

These two functions just update the member's properties and send them back to Discord to overwrite them. You could get this same effect by calling $member->deafen = true and following up with $guild->members->save($member), so it could be argued (correctly so) that this PR is inappropriate to begin with, and the logic should be changed to merely update the property and leave it up to the user to decide when the part should be saved. This is the same way you'd rename a channel, update the property and save it to the guild's channel repository.

I know right now it seems like there's a missing feature here, but the reality is some functions like un/deafening members don't actually have API endpoints associated with them. While I agree that we do need a better way to access the functionality from out of this specific context, I don't think adding aliases or unnecessary calls to the PATCH endpoint is the way to go about it. I've been considering adding traits to the classes that specifically handle updating parts to prep them for saving, but nothing is set in stone yet as I'm not sure how we would futureproof it.

@valzargaming
Copy link
Member Author

valzargaming commented Jan 17, 2025

I'm open for any suggestions on how to accomplish this. Looking things through some more I still think it makes more sense to scrap this PR completely.

Ideally a helper method should be made that would return a PromiseInterface so it can reject on a permissions error and resolve($this) on success. We'd need to set up something like setDeafAttribute, but the biggest roadblock is that the current implementation is hijacking the __set magic method which only allows for a void return... That brings us back to my original idea, creating new functions with PromiseInterface returns, and the original problem. I'm leaning towards something like ->setDeaf($bool): PromiseInterface and it matches code patterns already found elsewhere, such as $channel->setPermissions(): PromiseInterface, but it would lose the context needed to check for channel permissions.

The current best solution is to leave things as is. Developers will need to manually update the attributes and call $repository->save($part) where it's appropriate to do so.

@valzargaming
Copy link
Member Author

valzargaming commented Jan 17, 2025

The docs are surprisingly scarce as to how un/deafening and un/muting should work. For deafening, the only requirements are the bot having the DEAFEN_MEMBERS bitwise permission and for the member to currently be in a voice channel, necessitating the need to call the function within the context of a channel. Muting members is identical except that the bot is allowed to mute itself. The only way we're able to implement this properly at this time is with the original suggestion, else the Member part would need to accept a channel as a parameter, which could cause further confusion as it'd be possible to pass a channel the member isn't already in, and would break the library's patterns.

@valzargaming
Copy link
Member Author

This should be production ready now

@valzargaming valzargaming merged commit 9ddea44 into master Jan 17, 2025
1 check passed
@valzargaming valzargaming deleted the deafen-members branch January 17, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants