-
-
Notifications
You must be signed in to change notification settings - Fork 96
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.
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
base: develop
Are you sure you want to change the base?
Conversation
51f997f
to
2f14b7d
Compare
I haven't looked at the code yet but I have some suggestions for the functionality.
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. |
6b9a125
to
ed46d62
Compare
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. |
ed46d62
to
6c50157
Compare
application/src/main/java/org/togetherjava/tjbot/features/dynamicvc/DynamicVoiceListener.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/dynamicvc/DynamicVoiceListener.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/dynamicvc/DynamicVoiceListener.java
Outdated
Show resolved
Hide resolved
6c50157
to
4bca50e
Compare
Going to help work on this. |
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.
(partial review. still have to checkout the actual logic of the feature)
/** | ||
* @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); | ||
} |
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.
im not sure i follow what this does. who is join channel and who is left channel?
@@ -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)); |
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.
id rename the feature to DynamicVoiceChannelListener
, i.e. add Channel
to signal that it deals with managing channels, not voice itself
Btw what happens to the chat of a voice channel? Do we simply delete the history? In that case we should also provide an initial message in the chat informing about that |
On 8/10/25 20:20, Connor Schweighöfer wrote:
SquidXTV left a comment (Together-Java/TJ-Bot#1114)
Btw what happens to the chat of a voice channel? Do we simply delete the history? In that case we should also provide an initial message in the chat informing about that
Good catch on this! The bot should send a message for each channel
creation and notify the people who are connected to it that the texts
sent in this channel are prone to deletion.
We could also add a command which only works for these dynamically-made
channels that toggles the option of storing some sort of 'archive'
somewhere in the server as a text file that contains the history of
everything said in the channel.
Though I think that it would be too much effort, not a lot of members
would benefit from this - much better to give them a warning prior
instead. :-)
|
warning at the beginning of the chat history sounds reasonable. but potentially also annoying to get a "new message"-notification each time u create a new voice channel. gotta see how this feels in practice. deleting the channel with all its history is fine for me. those are throw-away-channels. |
On 8/14/25 09:33, Daniel Tischner wrote:
Zabuzard left a comment (Together-Java/TJ-Bot#1114)
but potentially also annoying to get a "new message"-notification each time u create a new voice channel. gotta see how this feels in practice.
This is a valid point Daniel, and thank you for bringing it up.
Considering the way the feature is currently designed, members can not
possibly get a notification from the warning message in question,
assuming we send it as soon as the channel is created and _not_ as soon
as a member joins it.
Unless it so happens that person A connects to 'Voice Channel 1' and as
'Voice Channel 2' is created (where the warning message will be sent in
our hypothetical scenario), person B joins at the same time, the members
will connect to these dynamic channels notifications-free.
Of course, this implies that we need to send the warning message to the
first dynamic channel in existence, which is something that the Discord
bot should easily be able to handle without any friction.
deleting the channel with all its history is fine for me. those are throw-away-channels.
That would also work perfectly fine. Keep in mind though that there is
a realistic chance that some people might not treat them as such.
Of course they could use their common sense and save their findings in
more important places, but ideally we would want to provide as least
friction as possible for those who decide to collaborate in the voice
channels.
Let me know what you think. :)
|
something that is problematic is that we cannot store message content outside of discord (for legal/privacy reasons). message content needs to stay inside discord. so if we want to archive something, that archive must be in discord and available to the users in a way that they can for example still delete their own messages if desired. i dont really see a practical solution to that. so best is probably to get rid of the channels id assume |
I would also say that there isn't a need for that in practice. Like check the current channel's chat history. They mostly consist of convos between people using mic vs people just listening and writing messages instead, so archiving that is mostly out of context. |
On 8/14/25 14:36, Daniel Tischner wrote:
something that is problematic is that we cannot store message content outside of discord (for legal/privacy reasons).
I agree with that. The warning message should only mention that the
messages are prone to deletion as it is an ephemeral voice channel.
It is unnecessary (and as you mentioned, illegal) to provide an option
for members to export a chat log of the voice channel they were in.
so if we want to archive something, that archive must be in discord and available to the users in a way that they can for example still delete their own messages if desired.
We could implement something like this in the future if there's demand
for it, but I highly doubt there will be. So for the time being, we
could just send a warning message inside these dynamic channels that the
messages are prone to deletion, and once everybody disconnects, just
delete that voice channel (and by extension, its chat history).
|
Closes #1113.
Media
Configuration changes
dynamicVoiceChannelPatterns
TODO