Add support for channel categories #907

LDSpits · 2017-12-13T20:25:15Z

This PR continues work from the (closed) #821 PR.

Credit for original work goes to Pegasy, However it looked like his work has gone stale so I've gone ahead and addressed the requested changes (and updated the branch).

Renamed all properties to use Category instead of Parent Throw exception on GetUsers / GetInvites etc for categories

… of ChannelCategory to be consistant with how text / voice channels are named.

…nto feature/channel-categories

AntiTcb · 2017-12-13T20:55:13Z

src/Discord.Net.Rest/Entities/Channels/RestCategoryChannel.cs

+    [DebuggerDisplay(@"{DebuggerDisplay,nq}")]
+    public class RestCategoryChannel : RestGuildChannel, ICategoryChannel
+    {
+        public string Mention => MentionUtils.MentionChannel(Id);


Probably shouldn't be implemented. Text Channels are the only channel types that have a full support for being mentioned.

AntiTcb · 2017-12-13T21:04:10Z

src/Discord.Net.WebSocket/Entities/Channels/SocketCategoryChannel.cs

+    {
+        public override IReadOnlyCollection<SocketGuildUser> Users
+            => Guild.Users.Where(x => x.VoiceChannel?.Id == Id).ToImmutableArray();
+


A Channels property should be implemented here since the cache is available to pull from.

@AntiTcb has been addressed, only card hasn't been closed since I added the feature under the selected line.

…operty to SocketCategoryChannel

LDSpits · 2017-12-17T11:06:16Z

Any more feedback for this pull request? @AntiTcb @foxbot

FiniteReality

Most of this looks fine, only a few style tweaks which I would make.

FiniteReality · 2017-12-20T13:17:06Z

src/Discord.Net.Rest/Entities/Channels/RestGuildChannel.cs

@@ -16,6 +16,8 @@ public class RestGuildChannel : RestChannel, IGuildChannel, IUpdateable
        internal IGuild Guild { get; }
        public string Name { get; private set; }
        public int Position { get; private set; }
+        public ulong? CategoryId { get; private set; }
+        public async Task<ICategoryChannel> GetCategoryAsync() => CategoryId == null ? null : await Guild.GetChannelAsync(CategoryId.Value).ConfigureAwait(false) as ICategoryChannel;


Probably best to put the body on a newline, to be consistent with the rest of the lib:

int MyMethod() => 5;

FiniteReality · 2017-12-20T13:21:17Z

src/Discord.Net.Rest/Entities/Guilds/RestGuild.cs

+            if (mode == CacheMode.AllowDownload)
+                return await GetCategoryChannelsAsync(options).ConfigureAwait(false);
+            else
+                return null;


Return an empty ImmutableArray here, as in GetVoiceChannels.

Furthermore, this method should be below GetVoiceChannelAsync and an accompanying GetCategoryAsync should be added.

Can also wrap the empty array in a Task.FromResult() and not await the other method so we avoid allocating the statemachine at runtime.

@Joe4evr looking at the other methods that return immutable arrays (GetVoiceChannelsAsync) I see that they don't even use Task.FromResult to return the immutable array. Ill just return a new instance. instead of wrapping it.

That's fair.

FiniteReality · 2017-12-20T13:22:50Z

src/Discord.Net.WebSocket/Entities/Channels/SocketGuildChannel.cs

@@ -17,6 +17,9 @@ public class SocketGuildChannel : SocketChannel, IGuildChannel
        public SocketGuild Guild { get; }
        public string Name { get; private set; }
        public int Position { get; private set; }
+        public ulong? CategoryId { get; private set; }
+        public ICategoryChannel Category => CategoryId == null ? null : Guild.GetChannel(CategoryId.Value) as ICategoryChannel;


Getter should probably be on a new line to increase clarity.

FiniteReality · 2017-12-20T13:23:11Z

src/Discord.Net.WebSocket/Entities/Channels/SocketGuildChannel.cs

@@ -17,6 +17,9 @@ public class SocketGuildChannel : SocketChannel, IGuildChannel
        public SocketGuild Guild { get; }
        public string Name { get; private set; }
        public int Position { get; private set; }
+        public ulong? CategoryId { get; private set; }
+        public ICategoryChannel Category => CategoryId == null ? null : Guild.GetChannel(CategoryId.Value) as ICategoryChannel;
+        Task<ICategoryChannel> IGuildChannel.GetCategoryAsync() => Task.FromResult(Category);


Method bodies should also be on a new line.

FiniteReality · 2017-12-20T13:24:03Z

src/Discord.Net.WebSocket/Entities/Guilds/SocketGuild.cs

@@ -645,6 +650,8 @@ Task<ITextChannel> IGuild.GetTextChannelAsync(ulong id, CacheMode mode, RequestO
            => Task.FromResult<ITextChannel>(GetTextChannel(id));
        Task<IReadOnlyCollection<IVoiceChannel>> IGuild.GetVoiceChannelsAsync(CacheMode mode, RequestOptions options)
            => Task.FromResult<IReadOnlyCollection<IVoiceChannel>>(VoiceChannels);
+        Task<IReadOnlyCollection<ICategoryChannel>> IGuild.GetCategoriesAsync(CacheMode mode , RequestOptions options)


Again, move this below GetVoiceChannelAsync and add GetCategoryAsync

foxbot · 2018-01-07T03:28:50Z

Thanks for your changes!

commit a85c5814a74e473e95fe172f0379cbc7f9f951d8 Author: Christopher F <computerizedtaco@gmail.com> Date: Sat Jan 6 22:25:48 2018 -0500 Code cleanup commit 4b243fd3dd99152b4ebc7ee01d704bd8e57eeee1 Author: Christopher F <computerizedtaco@gmail.com> Date: Sat Jan 6 22:08:28 2018 -0500 Add support for channel categories (#907) commit 41ed910 Author: mrspits4ever <spits.lucas@gmail.com> Date: Thu Dec 14 20:02:57 2017 +0100 removed mentioning support for RestCategoryChannel, added channels property to SocketCategoryChannel commit 71142c3 Merge: 4589d73 678a723 Author: mrspits4ever <spits.lucas@gmail.com> Date: Wed Dec 13 21:17:53 2017 +0100 Merge branch 'dev' of https://github.com/RogueException/Discord.Net into feature/channel-categories commit 4589d73 Author: mrspits4ever <spits.lucas@gmail.com> Date: Wed Dec 13 21:17:46 2017 +0100 adressed requested changes commit d59b038 Author: pegasy <pegasy@users.noreply.github.com> Date: Mon Sep 25 18:53:23 2017 +0200 Renamed classes / properties / methods to use CategoryChannel instead of ChannelCategory to be consistant with how text / voice channels are named. commit 5c4777d Author: pegasy <pegasy@users.noreply.github.com> Date: Sun Sep 24 19:08:25 2017 +0200 removed Guild from class name for ChannelCategory Renamed all properties to use Category instead of Parent Throw exception on GetUsers / GetInvites etc for categories commit e18bd8c Author: pegasy <pegasy@users.noreply.github.com> Date: Sun Sep 24 15:49:51 2017 +0200 Add support for channel categories (as its own channel type)

commit a85c5814a74e473e95fe172f0379cbc7f9f951d8 Author: Christopher F <computerizedtaco@gmail.com> Date: Sat Jan 6 22:25:48 2018 -0500 Code cleanup commit 4b243fd3dd99152b4ebc7ee01d704bd8e57eeee1 Author: Christopher F <computerizedtaco@gmail.com> Date: Sat Jan 6 22:08:28 2018 -0500 Add support for channel categories (discord-net#907) commit 41ed910 Author: mrspits4ever <spits.lucas@gmail.com> Date: Thu Dec 14 20:02:57 2017 +0100 removed mentioning support for RestCategoryChannel, added channels property to SocketCategoryChannel commit 71142c3 Merge: 4589d73 678a723 Author: mrspits4ever <spits.lucas@gmail.com> Date: Wed Dec 13 21:17:53 2017 +0100 Merge branch 'dev' of https://github.com/RogueException/Discord.Net into feature/channel-categories commit 4589d73 Author: mrspits4ever <spits.lucas@gmail.com> Date: Wed Dec 13 21:17:46 2017 +0100 adressed requested changes commit d59b038 Author: pegasy <pegasy@users.noreply.github.com> Date: Mon Sep 25 18:53:23 2017 +0200 Renamed classes / properties / methods to use CategoryChannel instead of ChannelCategory to be consistant with how text / voice channels are named. commit 5c4777d Author: pegasy <pegasy@users.noreply.github.com> Date: Sun Sep 24 19:08:25 2017 +0200 removed Guild from class name for ChannelCategory Renamed all properties to use Category instead of Parent Throw exception on GetUsers / GetInvites etc for categories commit e18bd8c Author: pegasy <pegasy@users.noreply.github.com> Date: Sun Sep 24 15:49:51 2017 +0200 Add support for channel categories (as its own channel type)

pegasy and others added 5 commits September 24, 2017 15:49

Add support for channel categories (as its own channel type)

e18bd8c

removed Guild from class name for ChannelCategory

5c4777d

Renamed all properties to use Category instead of Parent Throw exception on GetUsers / GetInvites etc for categories

Renamed classes / properties / methods to use CategoryChannel instead…

d59b038

… of ChannelCategory to be consistant with how text / voice channels are named.

adressed requested changes

4589d73

Merge branch 'dev' of https://github.com/RogueException/Discord.Net i…

71142c3

…nto feature/channel-categories

AntiTcb suggested changes Dec 13, 2017

View reviewed changes

removed mentioning support for RestCategoryChannel, added channels pr…

41ed910

…operty to SocketCategoryChannel

FiniteReality requested changes Dec 20, 2017

View reviewed changes

foxbot closed this Jan 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for channel categories #907

Add support for channel categories #907

LDSpits commented Dec 13, 2017

AntiTcb Dec 13, 2017

AntiTcb Dec 13, 2017

LDSpits Dec 14, 2017 •

edited

Loading

LDSpits commented Dec 17, 2017

FiniteReality left a comment

FiniteReality Dec 20, 2017

FiniteReality Dec 20, 2017

Joe4evr Dec 20, 2017 •

edited

Loading

LDSpits Dec 22, 2017

Joe4evr Dec 22, 2017

FiniteReality Dec 20, 2017

FiniteReality Dec 20, 2017

FiniteReality Dec 20, 2017

foxbot commented Jan 7, 2018

Add support for channel categories #907

Add support for channel categories #907

Conversation

LDSpits commented Dec 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LDSpits Dec 14, 2017 • edited Loading

Choose a reason for hiding this comment

LDSpits commented Dec 17, 2017

FiniteReality left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Joe4evr Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foxbot commented Jan 7, 2018

LDSpits Dec 14, 2017 •

edited

Loading

Joe4evr Dec 20, 2017 •

edited

Loading