make positions for channels and roles nicer (#1211)

* make role calculated position nicer

* make channels sortable in a nice way too

* stupid git web rebase editor

* Update Guild.js

* Update Guild.js

* Update Guild.js

* Update Guild.js

* Update RESTMethods.js

src/client/actions/GuildChannelsPositionUpdate.js

@@ -7,7 +7,7 @@ class GuildChannelsPositionUpdate extends Action {
     const guild = client.guilds.get(data.guild_id);
     if (guild) {
       for (const partialChannel of data.channels) {
-        const channel = guild.roles.get(partialChannel.id);
+        const channel = guild.channels.get(partialChannel.id);
         if (channel) channel.position = partialChannel.position;
       }
     }

src/client/actions/GuildRolesPositionUpdate.js

@@ -8,9 +8,7 @@ class GuildRolesPositionUpdate extends Action {
     if (guild) {
       for (const partialRole of data.roles) {
         const role = guild.roles.get(partialRole.id);
-        if (role) {
+        if (role) role.position = partialRole.position;
-          role.position = partialRole.position;
-        }
       }
     }
 

src/client/rest/RESTMethods.js

@@ -805,6 +805,15 @@ class RESTMethods {
     );
   }
 
+  setChannelPositions(guildID, channels) {
+    return this.rest.makeRequest('patch', Endpoints.Guild(guildID).channels, true, channels).then(() =>
+      this.client.actions.GuildChannelsPositionUpdate.handle({
+        guild_id: guildID,
+        channels,
+      }).guild
+    );
+  }
+
   addMessageReaction(message, emoji) {
     return this.rest.makeRequest(
       'put', Endpoints.Message(message).Reaction(emoji).User('@me'), true

src/structures/Guild.js

@@ -1,3 +1,4 @@
+const Long = require('long');
 const User = require('./User');
 const Role = require('./Role');
 const Emoji = require('./Emoji');
@@ -295,6 +296,15 @@ class Guild {
     return this.roles.get(this.id);
   }
 
+  /**
+   * Fetches a collection of roles in the current guild sorted by position.
+   * @type {Collection<Snowflake, Role>}
+   * @readonly
+   */
+  get _sortedRoles() {
+    return this._sortPositionWithID(this.roles);
+  }
+
   /**
    * Returns the GuildMember form of a User object, if the user is present in the guild.
    * @param {UserResolvable} user The user that you want to obtain the GuildMember of
@@ -687,31 +697,6 @@ class Guild {
     return this.client.rest.methods.createGuildRole(this, data);
   }
 
-  /**
-   * Set the position of a role in this guild
-   * @param {Role|Snowflake} role the role to edit, can be a role object or a role ID.
-   * @param {number} position the new position of the role
-   * @param {boolean} [relative=false] Position moves the role relative to its current position
-   * @returns {Promise<Guild>}
-   */
-  setRolePosition(role, position, relative = false) {
-    if (typeof role === 'string') {
-      role = this.roles.get(role);
-      if (!role) return Promise.reject(new Error('Supplied role is not a role or string.'));
-    }
-
-    position = Number(position);
-    if (isNaN(position)) return Promise.reject(new Error('Supplied position is not a number.'));
-
-    let updatedRoles = Object.assign([], this.roles.array()
-      .sort((r1, r2) => r1.position !== r2.position ? r1.position - r2.position : r1.id - r2.id));
-
-    Util.moveElementInArray(updatedRoles, role, position, relative);
-
-    updatedRoles = updatedRoles.map((r, i) => ({ id: r.id, position: i }));
-    return this.client.rest.methods.setRolePositions(this.id, updatedRoles);
-  }
-
   /**
    * Creates a new custom emoji in the guild.
    * @param {BufferResolvable|Base64Resolvable} attachment The image for the emoji.
@@ -919,6 +904,81 @@ class Guild {
     }
     this.presences.set(id, new Presence(presence));
   }
+
+  /**
+   * Set the position of a role in this guild
+   * @param {string|Role} role The role to edit, can be a role object or a role ID.
+   * @param {number} position The new position of the role
+   * @param {boolean} [relative=false] Position Moves the role relative to its current position
+   * @returns {Promise<Guild>}
+   */
+  setRolePosition(role, position, relative = false) {
+    if (typeof role === 'string') {
+      role = this.roles.get(role);
+      if (!role) return Promise.reject(new Error('Supplied role is not a role or snowflake.'));
+    }
+
+    position = Number(position);
+    if (isNaN(position)) return Promise.reject(new Error('Supplied position is not a number.'));
+
+    let updatedRoles = this._sortedRoles().array();
+
+    Util.moveElementInArray(updatedRoles, role, position, relative);
+
+    updatedRoles = updatedRoles.map((r, i) => ({ id: r.id, position: i }));
+    return this.client.rest.methods.setRolePositions(this.id, updatedRoles);
+  }
+
+  /**
+   * Set the position of a channel in this guild
+   * @param {string|GuildChannel} channel The channel to edit, can be a channel object or a channel ID.
+   * @param {number} position The new position of the channel
+   * @param {boolean} [relative=false] Position Moves the channel relative to its current position
+   * @returns {Promise<Guild>}
+   */
+  setChannelPosition(channel, position, relative = false) {
+    if (typeof channel === 'string') {
+      channel = this.channels.get(channel);
+      if (!channel) return Promise.reject(new Error('Supplied channel is not a channel or snowflake.'));
+    }
+
+    position = Number(position);
+    if (isNaN(position)) return Promise.reject(new Error('Supplied position is not a number.'));
+
+    let updatedChannels = this._sortedChannels(channel.type).array();
+
+    Util.moveElementInArray(updatedChannels, channel, position, relative);
+
+    updatedChannels = updatedChannels.map((r, i) => ({ id: r.id, position: i }));
+    return this.client.rest.methods.setChannelPositions(this.id, updatedChannels);
+  }
+
+  /**
+   * Fetches a collection of channels in the current guild sorted by position.
+   * @param {string} type Channel type
+   * @returns {Collection<Snowflake, GuildChannel>}
+   */
+  _sortedChannels(type) {
+    return this._sortPositionWithID(this.channels.filter(c => {
+      if (type === 'voice' && c.type === 'voice') return true;
+      else if (type !== 'voice' && c.type !== 'voice') return true;
+      else return type === c.type;
+    }));
+  }
+
+  /**
+   * Sorts a collection by object position or ID if the positions are equivalent.
+   * Intended to be identical to Discord's sorting method.
+   * @param {Collection} collection The collection to sort
+   * @returns {Collection}
+   */
+  _sortPositionWithID(collection) {
+    return collection.sort((a, b) =>
+      a.position !== b.position ?
+      a.position - b.position :
+      Long.fromString(a.id).sub(Long.fromString(b.id)).toNumber()
+    );
+  }
 }
 
 module.exports = Guild;

src/structures/GuildChannel.js

@@ -46,6 +46,15 @@ class GuildChannel extends Channel {
     }
   }
 
+  /**
+   * The position of the channel
+   * @type {number}
+   */
+  get calculatedPosition() {
+    const sorted = this.guild._sortedChannels(this.type);
+    return sorted.array().indexOf(sorted.get(this.id));
+  }
+
   /**
    * Gets the overall set of permissions for a user in this channel, taking into account roles and permission
    * overwrites.
@@ -220,6 +229,7 @@ class GuildChannel extends Channel {
   /**
    * Set a new position for the guild channel
    * @param {number} position The new position for the guild channel
+   * @param {boolean} [relative=false] Move the position relative to its current value
    * @returns {Promise<GuildChannel>}
    * @example
    * // set a new channel position
@@ -227,8 +237,8 @@ class GuildChannel extends Channel {
    *  .then(newChannel => console.log(`Channel's new position is ${newChannel.position}`))
    *  .catch(console.error);
    */
-  setPosition(position) {
+  setPosition(position, relative) {
-    return this.client.rest.methods.updateChannel(this, { position });
+    return this.guild.setChannelPosition(this, position, relative).then(() => this);
   }
 
   /**

src/structures/Role.js

@@ -128,9 +128,8 @@ class Role {
    * @type {number}
    */
   get calculatedPosition() {
-    const sorted = this.guild.roles.array()
+    const sorted = this.guild._sortedRoles();
-      .sort((r1, r2) => r1.position !== r2.position ? r1.position - r2.position : r1.id - r2.id);
+    return sorted.array().indexOf(sorted.get(this.id));
-    return sorted.indexOf(sorted.find(r => r.id === this.id));
   }
 
   /**

src/util/Collection.js

@@ -369,6 +369,18 @@ class Collection extends Map {
       return testVal !== value || (testVal === undefined && !collection.has(key));
     });
   }
+
+  /**
+   * The sort() method sorts the elements of a collection in place and returns the collection.
+   * The sort is not necessarily stable. The default sort order is according to string Unicode code points.
+   * @param {Function} [compareFunction] Specifies a function that defines the sort order.
+   * if omitted, the collection is sorted according to each character's Unicode code point value,
+   * according to the string conversion of each element.
+   * @returns {Collection}
+   */
+  sort(compareFunction = (x, y) => +(x > y) || +(x === y) - 1) {
+    return new Collection(Array.from(this.entries()).sort((a, b) => compareFunction(a[1], b[1], a[0], b[0])));
+  }
 }
 
 module.exports = Collection;