From 59da5e63221173536da3c25c9246ed9f40915bd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Spie=C3=9F?= Date: Tue, 1 Nov 2022 13:55:55 +0100 Subject: [PATCH] Improve GuildChannel#getPosition and Guild#getChannels --- .../attribute/IPositionableChannel.java | 30 +++---- .../jda/internal/entities/GuildImpl.java | 83 +++++-------------- .../middleman/AbstractGuildChannelImpl.java | 62 ++++++++++++-- 3 files changed, 90 insertions(+), 85 deletions(-) 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