Skip to content

Dynamically create and delete voice channels based on activity #1114

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Dynamically create and delete voice channels based on activity #1114

wants to merge 4 commits into from
+509 −2

Conversation

christolis
Copy link
Member

@christolis christolis commented May 14, 2024 •
edited
Loading

Closes #1113.

Media

output

Configuration changes

Property Description Type
dynamicVoiceChannelPatterns List of voice channel patterns to manage dynamically. List[String]

@christolis christolis added the enhancement New feature or request label May 14, 2024
@christolis christolis self-assigned this May 14, 2024
@christolis christolis force-pushed the feat/dynamic-vcs branch 2 times, most recently from 51f997f to 2f14b7d Compare May 15, 2024 14:35
@christolis christolis marked this pull request as ready for review May 15, 2024 15:27
@christolis christolis requested a review from a team as a code owner May 15, 2024 15:27
@tj-wazei
Copy link

I haven't looked at the code yet but I have some suggestions for the functionality.

  • We should have a new Voice Chat that's more descriptive on "hey you're about to create a new voice channel". Maybe something like "New Voice Channel" instead of using Chit Chat.
  • The new room should be based on the "creators" name e.g. "Christolis's Room" and it should automatically move you from the "New Voice Channel" to "Christolis's Room" (people can rename after as needed)
  • We should have a cleanup task e.g. if a voice channel has been empty for 5 minutes then it should be deleted.

Importantly, the existing channels should be protected from the changes that get implemented here.

This is just an IMO - we should wait for further feedback first.

@christolis christolis marked this pull request as draft May 15, 2024 21:29
@christolis christolis marked this pull request as ready for review May 16, 2024 08:26
@ankitsmt211
Copy link
Member

I love @tj-wazei suggestions, we shouldnt delete channels immediately and one more thing to add. We also wanna cap channel count to a certain limit. For example we can't have more than say 3 channels per category for now. We can put this limit in configuration so it's easily tweaked without making changes to code by maintainers.

@christolis christolis requested a review from SquidXTV October 29, 2024 19:10
@ZahirHumphries
Copy link

Going to help work on this.

@Zabuzard Zabuzard added priority: normal config-changes if your PR contains any changes related to config file labels Jul 1, 2025
Zabuzard
Zabuzard requested changes Jul 1, 2025
View reviewed changes
Copy link
Member

@Zabuzard Zabuzard left a comment

Choose a reason for hiding this comment

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

(partial review. still have to checkout the actual logic of the feature)

application/src/main/java/org/togetherjava/tjbot/features/system/BotCore.java
Comment on lines +258 to +271
/**
* @param joinChannel the join channel
* @param leftChannel the leave channel
* @return the join channel if not null, otherwise the leave channel, otherwise an empty
* optional
*/
private Optional<Channel> calculateSubscribeTarget(@Nullable AudioChannelUnion joinChannel,
@Nullable AudioChannelUnion leftChannel) {
if (joinChannel != null) {
return Optional.of(joinChannel);
}

return Optional.ofNullable(leftChannel);
}
Copy link
Member

Choose a reason for hiding this comment

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

im not sure i follow what this does. who is join channel and who is left channel?

application/src/main/java/org/togetherjava/tjbot/features/Features.java
@@ -151,6 +152,9 @@ public static Collection<Feature> createFeatures(JDA jda, Database database, Con
features.add(new SlashCommandEducator());
features.add(new PinnedNotificationRemover(config));

// Voice receivers
features.add(new DynamicVoiceListener(config));
Copy link
Member

Choose a reason for hiding this comment

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

id rename the feature to DynamicVoiceChannelListener, i.e. add Channel to signal that it deals with managing channels, not voice itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zabuzard Zabuzard Zabuzard requested changes

@SquidXTV SquidXTV Awaiting requested review from SquidXTV

Requested changes must be addressed to merge this pull request.

Assignees

@christolis christolis

@ZahirHumphries ZahirHumphries

Labels
config-changes if your PR contains any changes related to config file enhancement New feature or request priority: normal
Projects
Together Java
Status: In Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dynamically create and delete voice channels based on activity
6 participants
@christolis @tj-wazei @ankitsmt211 @ZahirHumphries @Zabuzard @SquidXTV