diff --git a/src/main/java/net/dv8tion/jda/api/entities/channel/attribute/IPositionableChannel.java b/src/main/java/net/dv8tion/jda/api/entities/channel/attribute/IPositionableChannel.java
index 625099e86b..9a1832ad28 100644
--- a/src/main/java/net/dv8tion/jda/api/entities/channel/attribute/IPositionableChannel.java
+++ b/src/main/java/net/dv8tion/jda/api/entities/channel/attribute/IPositionableChannel.java
@@ -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.
@@ -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.
- *
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.
+ *
This does not account for thread channels, as they do not have positions.
+ *
+ *
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
@@ -52,23 +51,16 @@ public interface IPositionableChannel extends GuildChannel
*/
default int getPosition()
{
- int sortBucket = getType().getSortBucket();
- List 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)
+ *
+ * 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
diff --git a/src/main/java/net/dv8tion/jda/internal/entities/GuildImpl.java b/src/main/java/net/dv8tion/jda/internal/entities/GuildImpl.java
index 5eaedba630..3dcb4d8dd3 100644
--- a/src/main/java/net/dv8tion/jda/internal/entities/GuildImpl.java
+++ b/src/main/java/net/dv8tion/jda/internal/entities/GuildImpl.java
@@ -747,72 +747,33 @@ public SnowflakeCacheView getStickerCache()
public List getChannels(boolean includeHidden)
{
Member self = getSelfMember();
- Predicate filterHidden = it -> self.hasPermission(it, Permission.VIEW_CHANNEL);
-
- List channels;
- SnowflakeCacheViewImpl categoryView = getCategoriesView();
- SnowflakeCacheViewImpl voiceView = getVoiceChannelsView();
- SnowflakeCacheViewImpl stageView = getStageChannelsView();
- SnowflakeCacheViewImpl textView = getTextChannelsView();
- SnowflakeCacheViewImpl newsView = getNewsChannelView();
- SnowflakeCacheViewImpl forumView = getForumChannelsView();
- List textChannels;
- List newsChannels;
- List voiceChannels;
- List stageChannels;
- List forumChannels;
- List 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 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 categories = getCategoriesView();
+ SnowflakeCacheViewImpl voice = getVoiceChannelsView();
+ SnowflakeCacheViewImpl stage = getStageChannelsView();
+ SnowflakeCacheViewImpl text = getTextChannelsView();
+ SnowflakeCacheViewImpl news = getNewsChannelView();
+ SnowflakeCacheViewImpl forum = getForumChannelsView();
+
+ List 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 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);
}
diff --git a/src/main/java/net/dv8tion/jda/internal/entities/channel/middleman/AbstractGuildChannelImpl.java b/src/main/java/net/dv8tion/jda/internal/entities/channel/middleman/AbstractGuildChannelImpl.java
index bef866ac55..c24058af19 100644
--- a/src/main/java/net/dv8tion/jda/internal/entities/channel/middleman/AbstractGuildChannelImpl.java
+++ b/src/main/java/net/dv8tion/jda/internal/entities/channel/middleman/AbstractGuildChannelImpl.java
@@ -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;
@@ -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