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

Improve GuildChannel#getPosition and Guild#getChannels #2320

Merged
merged 1 commit into from Nov 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -20,8 +20,6 @@
import net.dv8tion.jda.api.managers.channel.attribute.IPositionableChannelManager;

import javax.annotation.Nonnull;
import java.util.List;
import java.util.stream.Collectors;

/**
* Represents a {@link GuildChannel GuildChannel} that has a position.
Expand All @@ -37,13 +35,14 @@ public interface IPositionableChannel extends GuildChannel
@Nonnull
IPositionableChannelManager<?, ?> getManager();

//TODO-v5: We should probably reconsider how getPosition is calculated as it isn't particularly useful anymore...
//TODO-v5: We should probably introduce getPositionAbsolute that returns the index of the channel in Guild#getChannels
//TODO-v5: We should probably introduce getPositionInCategory (name pending) that returns index in Category#getChannels or -1

/**
* The position this GuildChannel is displayed at.
* <br>Higher values mean they are displayed lower in the Client. Position 0 is the top most GuildChannel
* Channels of a {@link net.dv8tion.jda.api.entities.Guild Guild} do not have to have continuous positions
* The position of this channel in the channel list of the guild.
* <br>This does not account for thread channels, as they do not have positions.
*
* <p>This is functionally equivalent to {@code getGuild().getChannels().indexOf(channel)}.
* To efficiently compare the position between channels, it is recommended to use {@link #compareTo(Object)} instead of the position.
*
* @throws IllegalStateException
* If this channel is not in the guild cache
Expand All @@ -52,23 +51,16 @@ public interface IPositionableChannel extends GuildChannel
*/
default int getPosition()
{
int sortBucket = getType().getSortBucket();
List<GuildChannel> channels = getGuild().getChannels().stream()
.filter(chan -> chan.getType().getSortBucket() == sortBucket)
.sorted()
.collect(Collectors.toList());

for (int i = 0; i < channels.size(); i++)
{
if (equals(channels.get(i)))
return i;
}
int position = getGuild().getChannels().indexOf(this);
if (position > -1)
return position;
throw new IllegalStateException("Somehow when determining position we never found the " + getType().name() + " in the Guild's channels? wtf?");
}

/**
* The actual position of the {@link GuildChannel GuildChannel} as stored and given by Discord.
* Channel positions are actually based on a pairing of the creation time (as stored in the snowflake id)
*
* <p>Channel positions are actually based on a pairing of the creation time (as stored in the snowflake id)
* and the position. If 2 or more channels share the same position then they are sorted based on their creation date.
* The more recent a channel was created, the lower it is in the hierarchy. This is handled by {@link #getPosition()}
* and it is most likely the method you want. If, for some reason, you want the actual position of the
Expand Down
83 changes: 22 additions & 61 deletions src/main/java/net/dv8tion/jda/internal/entities/GuildImpl.java
Expand Up @@ -747,72 +747,33 @@ public SnowflakeCacheView<GuildSticker> getStickerCache()
public List<GuildChannel> getChannels(boolean includeHidden)
{
Member self = getSelfMember();
Predicate<GuildChannel> filterHidden = it -> self.hasPermission(it, Permission.VIEW_CHANNEL);

List<GuildChannel> channels;
SnowflakeCacheViewImpl<Category> categoryView = getCategoriesView();
SnowflakeCacheViewImpl<VoiceChannel> voiceView = getVoiceChannelsView();
SnowflakeCacheViewImpl<StageChannel> stageView = getStageChannelsView();
SnowflakeCacheViewImpl<TextChannel> textView = getTextChannelsView();
SnowflakeCacheViewImpl<NewsChannel> newsView = getNewsChannelView();
SnowflakeCacheViewImpl<ForumChannel> forumView = getForumChannelsView();
List<TextChannel> textChannels;
List<NewsChannel> newsChannels;
List<VoiceChannel> voiceChannels;
List<StageChannel> stageChannels;
List<ForumChannel> forumChannels;
List<Category> categories;
try (UnlockHook categoryHook = categoryView.readLock();
UnlockHook voiceHook = voiceView.readLock();
UnlockHook textHook = textView.readLock();
UnlockHook newsHook = newsView.readLock();
UnlockHook stageHook = stageView.readLock();
UnlockHook forumHook = forumView.readLock())
{
if (includeHidden)
{
textChannels = textView.asList();
newsChannels = newsView.asList();
voiceChannels = voiceView.asList();
stageChannels = stageView.asList();
forumChannels = forumView.asList();
}
else
{
textChannels = textView.stream().filter(filterHidden).collect(Collectors.toList());
newsChannels = newsView.stream().filter(filterHidden).collect(Collectors.toList());
voiceChannels = voiceView.stream().filter(filterHidden).collect(Collectors.toList());
stageChannels = stageView.stream().filter(filterHidden).collect(Collectors.toList());
forumChannels = forumView.stream().filter(filterHidden).collect(Collectors.toList());
}
categories = categoryView.asList(); // we filter categories out when they are empty (no visible channels inside)
channels = new ArrayList<>((int) categoryView.size() + voiceChannels.size() + textChannels.size() + newsChannels.size() + stageChannels.size());
}
Predicate<GuildChannel> filterHidden = includeHidden ? (it) -> true : it -> self.hasPermission(it, Permission.VIEW_CHANNEL);

textChannels.stream().filter(it -> it.getParentCategory() == null).forEach(channels::add);
newsChannels.stream().filter(it -> it.getParentCategory() == null).forEach(channels::add);
voiceChannels.stream().filter(it -> it.getParentCategory() == null).forEach(channels::add);
stageChannels.stream().filter(it -> it.getParentCategory() == null).forEach(channels::add);
forumChannels.stream().filter(it -> it.getParentCategory() == null).forEach(channels::add);
Collections.sort(channels);
SnowflakeCacheViewImpl<Category> categories = getCategoriesView();
SnowflakeCacheViewImpl<VoiceChannel> voice = getVoiceChannelsView();
SnowflakeCacheViewImpl<StageChannel> stage = getStageChannelsView();
SnowflakeCacheViewImpl<TextChannel> text = getTextChannelsView();
SnowflakeCacheViewImpl<NewsChannel> news = getNewsChannelView();
SnowflakeCacheViewImpl<ForumChannel> forum = getForumChannelsView();

List<GuildChannel> channels = new ArrayList<>((int) (categories.size() + voice.size() + stage.size() + text.size() + news.size() + forum.size()));

voice.acceptStream(stream -> stream.filter(filterHidden).forEach(channels::add));
stage.acceptStream(stream -> stream.filter(filterHidden).forEach(channels::add));
text.acceptStream(stream -> stream.filter(filterHidden).forEach(channels::add));
news.acceptStream(stream -> stream.filter(filterHidden).forEach(channels::add));
forum.acceptStream(stream -> stream.filter(filterHidden).forEach(channels::add));

for (Category category : categories)
categories.forEach(category ->
{
List<GuildChannel> children;
if (includeHidden)
{
children = category.getChannels();
}
else
{
children = category.getChannels().stream().filter(filterHidden).collect(Collectors.toList());
if (children.isEmpty())
continue;
}
if (!includeHidden && category.getChannels().stream().noneMatch(filterHidden))
return;

channels.add(category);
channels.addAll(children);
}
});

// See AbstractGuildChannelImpl#compareTo for details on how this achieves the canonical order of the client
Collections.sort(channels);

return Collections.unmodifiableList(channels);
}
Expand Down
Expand Up @@ -16,7 +16,10 @@

package net.dv8tion.jda.internal.entities.channel.middleman;

import net.dv8tion.jda.api.entities.channel.attribute.ICategorizableChannel;
import net.dv8tion.jda.api.entities.channel.attribute.IPositionableChannel;
import net.dv8tion.jda.api.entities.channel.concrete.Category;
import net.dv8tion.jda.api.entities.channel.concrete.ThreadChannel;
import net.dv8tion.jda.api.entities.channel.middleman.GuildChannel;
import net.dv8tion.jda.internal.entities.GuildImpl;
import net.dv8tion.jda.internal.entities.channel.AbstractChannelImpl;
Expand Down Expand Up @@ -47,18 +50,67 @@ public int compareTo(@Nonnull GuildChannel o)
{
Checks.notNull(o, "Channel");

// if bucket matters
// Check thread positions
ThreadChannel thisThread = this instanceof ThreadChannel ? (ThreadChannel) this : null;
ThreadChannel otherThread = o instanceof ThreadChannel ? (ThreadChannel) o : null;

if (thisThread != null && otherThread == null)
return thisThread.getParentChannel().compareTo(o);
if (thisThread == null && otherThread != null)
return this.compareTo(otherThread.getParentChannel());
if (thisThread != null)
{
// If they are threads on the same channel
if (thisThread.getParentChannel().equals(otherThread.getParentChannel()))
return Long.compare(o.getIdLong(), id); // threads are ordered ascending by age
// If they are threads on different channels
return thisThread.getParentChannel().compareTo(otherThread.getParentChannel());
}

// Check category positions
Category thisParent = this instanceof ICategorizableChannel ? ((ICategorizableChannel) this).getParentCategory() : null;
Category otherParent = o instanceof ICategorizableChannel ? ((ICategorizableChannel) o).getParentCategory() : null;

if (thisParent != null && otherParent == null)
{
if (o instanceof Category)
{
// The other channel is the parent category of this channel
if (o.equals(thisParent))
return 1;
// The other channel is another category
return thisParent.compareTo(o);
}
return 1;
}
if (thisParent == null && otherParent != null)
{
if (this instanceof Category)
{
// This channel is parent of other channel
if (this.equals(otherParent))
return -1;
// This channel is a category higher than the other channel's parent category
return this.compareTo(otherParent); //safe use of recursion since no circular parents exist
}
return -1;
}
// Both channels are in different categories, compare the categories instead
if (thisParent != null && !thisParent.equals(otherParent))
return thisParent.compareTo(otherParent);

// Check sort bucket (text/message is above audio)
if (getType().getSortBucket() != o.getType().getSortBucket())
return Integer.compare(getType().getSortBucket(), o.getType().getSortBucket());

// if position matters
if (o instanceof IPositionableChannel && this instanceof IPositionableChannel) {
// Check actual position
if (o instanceof IPositionableChannel && this instanceof IPositionableChannel)
{
IPositionableChannel oPositionableChannel = (IPositionableChannel) o;
IPositionableChannel thisPositionableChannel = (IPositionableChannel) this;

if (thisPositionableChannel.getPositionRaw() != oPositionableChannel.getPositionRaw()) {
if (thisPositionableChannel.getPositionRaw() != oPositionableChannel.getPositionRaw())
return Integer.compare(thisPositionableChannel.getPositionRaw(), oPositionableChannel.getPositionRaw());
}
}

// last resort by id
Expand Down