Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@
"command-description": "Request a Custom role as a reward for boosting. This has a cooldown of 24 hours",
"manage-subcommand-description": "Create or edit your custom role",
"name-option-description": "The name of your custom role",
"color-option-description": "The color of your custom role",
"primary-color-option-description": "The (primary) color of your custom role",
"secondary-color-option-description": "The secondary color of your custom role. (If unlocked)",
"holographic-option-description": "If enabled, your role will use the holographic effect. (If unlocked)",
"remove-subcommand-description": "Remove your custom role",
"icon-option-description": "Your role-icon",
"confirm-option-remove-description": "Do you really want to delete your custom role? This will not reset any running cooldowns"
Expand Down
126 changes: 95 additions & 31 deletions modules/color-me/commands/color-me.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const {localize} = require('../../../src/functions/localize');
const {client} = require('../../../main');
const {embedType, dateToDiscordTimestamp} = require('../../../src/functions/helpers');
const { Constants } = require('discord.js');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: const { Constants } here vs const {Constants} in guildMemberUpdate.js:2 and the rest of the codebase. @stylistic/object-curly-spacing is off so lint won't catch it.


module.exports.beforeSubcommand = async function (interaction) {
await interaction.deferReply({ephemeral: true});
Expand All @@ -10,6 +11,7 @@ module.exports.subcommands = {
'manage': async function (interaction) {
let roleIcon;
let iconW = true;
let colorfulW = true;
if (interaction.options.getAttachment('icon') !== null) {
if (client.guild.features.includes('ROLE_ICONS')) {
roleIcon = interaction.options.getAttachment('icon').url;
Expand All @@ -22,6 +24,11 @@ module.exports.subcommands = {
const moduleStrings = interaction.client.configurations['color-me']['strings'];
const moduleModel = interaction.client.models['color-me']['Role'];

const multiColor = client.guild.features.includes('ENHANCED_ROLE_COLORS') && moduleConf['allowEnhancedRoleColors'];
if (!multiColor && (interaction.options.getString('secondary-color') !== null || interaction.options.getBoolean('holographic'))) {
colorfulW = false;
}

const pos = moduleConf.rolePosition
? interaction.guild.roles.resolve(moduleConf.rolePosition).position
: 0;
Expand All @@ -44,29 +51,62 @@ module.exports.subcommands = {
userID: interaction.user.id
}
});
const {
roleColor: primaryColor,
cancel
} = await color(interaction.options.getString('primary-color'), interaction, moduleStrings);

let secondaryColor, cancelSec;
if (multiColor) {
let {
roleColor: secondaryColorTemp,
cancel: cancelSecTemp
} = await color(interaction.options.getString('secondary-color'), interaction, moduleStrings);
secondaryColor = secondaryColorTemp;
cancelSec = cancelSecTemp;
} else {
secondaryColor = null;
}
if (cancel || cancelSec) return;
const isHolographic = interaction.options.getBoolean('holographic') && multiColor;
if (role) {
role = role.roleID;
const {
roleColor,
cancel
} = await color(interaction, moduleStrings);
if (cancel) return;
if (interaction.guild.roles.cache.find(r => r.id === role)) {
role = interaction.guild.roles.resolve(role);
role.edit(
await role.edit(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking - role is never patched by edit(), so the write-back below saves stale values.

RoleManager#edit returns a clone and leaves the original untouched (discord.js/src/managers/RoleManager.js:295-297):

const clone = role._clone();
clone._patch(d);
return clone;

and _clone() is Object.assign(Object.create(this), this), so _patch writes a fresh colors object onto the clone only.

That means at line 96-99 role.name and role.colors are still the pre-edit values, and the row records what the role looked like before the edit - which is exactly the staleness the write-back was added to fix. (The cached role does get patched eventually by the GUILD_ROLE_UPDATE gateway event, but that's a race, not a guarantee, and it definitely hasn't landed by the next line.)

Assigning the return value fixes it:

Suggested change
await role.edit(
role = await role.edit(

{
name: interaction.options.getString('name'),
color: roleColor,
colors: isHolographic ? {
primaryColor: Constants.HolographicStyle.Primary,
secondaryColor: Constants.HolographicStyle.Secondary,
tertiaryColor: Constants.HolographicStyle.Tertiary
} : {
primaryColor: primaryColor,
secondaryColor: secondaryColor
},
icon: roleIcon,
reason: localize('color-me', 'edit-log-reason', {
user: interaction.user.username
})
}
);
Comment thread
hfgd123 marked this conversation as resolved.
if (iconW) {
await moduleModel.update({
userID: interaction.user.id,
roleID: role.id,
name: role.name,
primaryColor: role.colors.primaryColor,
secondaryColor: role.colors.secondaryColor,
holo: !!role.colors.tertiaryColor,
Comment on lines +97 to +99

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking - these write raw integers into a STRING column, and the recreate path can't read them back.

role.colors.primaryColor is data.colors.primary_color, a raw integer (discord.js/src/structures/Role.js:82) - not a hex string. The old code stored role.hexColor. The column has TEXT affinity, so SQLite coerces on write and hands back a decimal string:

read back: { primaryColor: '11127295', secondaryColor: '16759788', holo: 1 }
resolveColor('11127295') -> DiscordjsTypeError [ColorConvert]: Unable to convert "11127295" to a number.

guildMemberUpdate.js:49-50 feeds that value straight into roles.create({colors}), which calls resolveColor() on it. So with recreateRole enabled, any role created or edited after this PR throws on the unboost -> reboost recreate.

It also splits the column into two formats: rows migrated from the old color column keep hex (I ran the migration - #f1c40f survives intact), new rows get decimals.

Storing hex keeps the column single-format and needs no value migration. There's only a hexColor getter for the primary, so it needs a small helper - which is why this isn't a one-click suggestion:

const toHex = (c) => (c === null || c === undefined ? null : '#' + c.toString(16).padStart(6, '0'));

then primaryColor: toHex(role.colors.primaryColor), secondaryColor: toHex(role.colors.secondaryColor). Same three lines repeat at 140-142 and 185-187.

timestamp: new Date()
}, {
where: {
userID: interaction.user.id
}
});
if (iconW && colorfulW) {
await interaction.editReply(embedType(moduleStrings['updated'], {}));
} else {
await interaction.editReply(embedType(moduleStrings['updatedNoIcon'], {}));
await interaction.editReply(embedType(moduleStrings['updatedLimited'], {}));
}
} else {
if (interaction.guild.roles.cache.size >= 250) {
Expand All @@ -76,7 +116,14 @@ module.exports.subcommands = {
role = await interaction.guild.roles.create(
{
name: interaction.options.getString('name'),
color: roleColor,
colors: isHolographic ? {
primaryColor: Constants.HolographicStyle.Primary,
secondaryColor: Constants.HolographicStyle.Secondary,
tertiaryColor: Constants.HolographicStyle.Tertiary
} : {
primaryColor: primaryColor,
secondaryColor: secondaryColor
},
icon: roleIcon,
hoist: moduleConf.listRoles,
permissions: '',
Expand All @@ -85,13 +132,14 @@ module.exports.subcommands = {
reason: localize('color-me', 'create-log-reason', {
user: interaction.user.username
})
}
);
});
await moduleModel.update({
userID: interaction.user.id,
roleID: role.id,
name: role.name,
color: role.hexColor,
primaryColor: role.colors.primaryColor,
secondaryColor: role.colors.secondaryColor,
holo: !!role.colors.tertiaryColor,
Comment on lines +140 to +142

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same integer-vs-hex problem as line 97 - this is the create-after-record-exists path.

timestamp: new Date()
}, {
where: {
Expand All @@ -101,23 +149,25 @@ module.exports.subcommands = {
if (!interaction.member.roles.cache.has(role)) {
await interaction.member.roles.add(role);
}
if (iconW) {
if (iconW && colorfulW) {
await interaction.editReply(embedType(moduleStrings['updated'], {}));
} else {
await interaction.editReply(embedType(moduleStrings['updatedNoIcon'], {}));
await interaction.editReply(embedType(moduleStrings['updatedLimited'], {}));
}
}
} else {
const {
roleColor,
cancel
} = await color(interaction, moduleStrings);
if (cancel) return;
try {
role = await interaction.guild.roles.create(
{
name: interaction.options.getString('name'),
color: roleColor,
colors: isHolographic ? {
primaryColor: Constants.HolographicStyle.Primary,
secondaryColor: Constants.HolographicStyle.Secondary,
tertiaryColor: Constants.HolographicStyle.Tertiary
} : {
primaryColor: primaryColor,
secondaryColor: secondaryColor
},
icon: roleIcon,
hoist: moduleConf.listRoles,
permissions: '',
Expand All @@ -132,11 +182,13 @@ module.exports.subcommands = {
userID: interaction.user.id,
roleID: role.id,
name: role.name,
color: role.hexColor,
primaryColor: role.colors.primaryColor,
secondaryColor: role.colors.secondaryColor,
holo: !!role.colors.tertiaryColor,
Comment on lines +185 to +187

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same integer-vs-hex problem again - the first-time create path.

timestamp: new Date()
});
await interaction.member.roles.add(role);
if (iconW) {
if (iconW && colorfulW) {
await interaction.editReply(embedType(moduleStrings['created'], {}));
} else {
await interaction.editReply(embedType(moduleStrings['createdNoIcon'], {}));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updatedNoIcon was renamed to updatedLimited to cover both features, but the create path still falls through to createdNoIcon, whose text is unchanged:

Your role was created successfully, but your role icon was not used, as this requires the guild to be boost level 2 or higher.

So a user who asks for holographic on a non-enhanced guild is told their icon was dropped. createdNoIcon needs the same rename + rewording treatment as updatedLimited in strings.json.

Expand Down Expand Up @@ -168,7 +220,7 @@ module.exports.subcommands = {
role = role.roleID;
if (interaction.guild.roles.cache.find(r => r.id === role)) {
role = interaction.guild.roles.resolve(role);
role.delete(localize('color-me', 'delete-manual-log-reason', {
await role.delete(localize('color-me', 'delete-manual-log-reason', {
user: interaction.member.user.username
}));
await interaction.editReply(await embedType(moduleStrings['removed'], {}));
Expand Down Expand Up @@ -196,8 +248,20 @@ module.exports.config = {
{
type: 'STRING',
required: false,
name: 'color',
description: localize('color-me', 'color-option-description')
name: 'primary-color',
description: localize('color-me', 'primary-color-option-description')
},
{
type: 'STRING',
required: false,
name: 'secondary-color',
description: localize('color-me', 'secondary-color-option-description')
},
{
type: 'BOOLEAN',
required: false,
name: 'holographic',
description: localize('color-me', 'holographic-option-description')
},
{
type: 'ATTACHMENT',
Expand Down Expand Up @@ -227,9 +291,9 @@ module.exports.config = {
* Gets a color from the String of a command option
* @returns {Promise<{roleColor: string|number, cancel: boolean}>}
*/
async function color(interaction, moduleStrings) {
if (interaction.options.getString('color')) {
let roleColor = interaction.options.getString('color');
async function color(colorString, interaction, moduleStrings) {
if (colorString) {
let roleColor = colorString;
if (!roleColor.startsWith('#')) {
roleColor = '#' + roleColor;
}
Expand All @@ -246,12 +310,12 @@ async function color(interaction, moduleStrings) {
};
}
return {
roleColor: 0xF1C40F,
roleColor: 0x000000,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flips the default from gold to colorless for every role created or edited without primary-color - and since manage always sends colors, that includes someone editing only their role name.

It's arguably the better default (Discord renders 0 as "no color" rather than actual black), but it's a silent behaviour change for every existing server, so worth being deliberate about it rather than letting it ride in as part of the gradient work.

cancel: false
};
}

// Exported for unit testing of the colour-validation logic.
// Exported for unit testing of the color-validation logic.
module.exports.color = color;

/**
Expand Down
7 changes: 7 additions & 0 deletions modules/color-me/configs/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
"default": "",
"description": "The role, beneath which the custom-roles should be created",
"type": "roleID"
},
{
"name": "allowEnhancedRoleColors",
"humanName": "Allow \"Enhanced Role Colors\"",
"default": true,
"description": "Should the module allow users to use the \"Enhanced Role Colors\" feature? (If your server doesn't have this feature unlocked, this setting will have no effect)",
"type": "boolean"
}
Comment on lines +41 to 47

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config-localizations/en.json needs regenerating - it's the generated Weblate reference, and at this head it still carries the old updatedNoIcon entry and has no entry for allowEnhancedRoleColors or updatedLimited.

Nothing crashes (getLocalizedConfig falls back to the inline English when a key is missing), but the new field and the new string can never be translated, and translators keep seeing a dead string.

node config-localizations/generate-files.js

]
}
8 changes: 4 additions & 4 deletions modules/color-me/configs/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
"allowEmbed": true
},
{
"name": "updatedNoIcon",
"humanName": "Role updated without icon",
"default": "Your role was updated successfully, but your role icon was not used, as this requires the guild to be boost level 2 or higher.",
"description": "This messages gets send when a booster sucessfully updates their custom role, but the guild has not enough boosts to use role icons",
"name": "updatedLimited",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads-up rather than a change request: renaming the key means any server that customised updatedNoIcon silently loses that text and reverts to this new default (configuration.js falls back to the field default when a stored key is absent). Fine given the meaning genuinely changed - just a one-way reset worth knowing about.

"humanName": "Role updated with limited features",
"default": "Your role was updated successfully, but either your role icon or enhanced role colors were not used, as these features either need to be unlocked by boosting the server or were disabled by the server admin.",
"description": "This message is sent when a booster successfully updates their custom role, but the guild does not have enough boosts to use role icons or enhanced role colors, or the feature was disabled in the module configuration",
"type": "string",
"allowEmbed": true
},
Expand Down
16 changes: 13 additions & 3 deletions modules/color-me/events/guildMemberUpdate.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const {localize} = require('../../../src/functions/localize');
const {Constants} = require('discord.js');
let pos;

module.exports.run = async function (client, oldGuildMember, newGuildMember) {
Expand Down Expand Up @@ -36,7 +37,7 @@ module.exports.run = async function (client, oldGuildMember, newGuildMember) {
if (moduleConf.recreateRole) {
if (!oldGuildMember.premiumSince && newGuildMember.premiumSince) {
const data = await client.models['color-me']['Role'].findOne({
attributes: ['roleID', 'name', 'color'],
attributes: ['roleID', 'name', 'primaryColor', 'secondaryColor', 'holo'],
raw: true,
where: {
userID: newGuildMember.id
Expand All @@ -45,12 +46,21 @@ module.exports.run = async function (client, oldGuildMember, newGuildMember) {
if (data) {
let role = data.roleID;
const name = data.name;
const color = data.color;
const primaryColor = data.primaryColor;
const secondaryColor = data.secondaryColor;
const isHolographic = data.holo && client.guild.features.includes('ENHANCED_ROLE_COLORS');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowEnhancedRoleColors got folded into multiColor in color-me.js, but the event path still checks only the guild feature. So an admin who turns the setting off still gets holographic roles recreated on reboost - the switch is still half-inert.

moduleConf is already in scope from line 10, and hoisting the check into a local also cleans up the duplicate features.includes() on line 62:

Suggested change
const isHolographic = data.holo && client.guild.features.includes('ENHANCED_ROLE_COLORS');
const enhancedColors = client.guild.features.includes('ENHANCED_ROLE_COLORS') && moduleConf['allowEnhancedRoleColors'];
const isHolographic = data.holo && enhancedColors;

if (!newGuildMember.guild.roles.cache.find(r => r.id === role)) {
role = await client.guild.roles.create(
{
name: name,
color: color,
colors: isHolographic ? {
primaryColor: Constants.HolographicStyle.Primary,
secondaryColor: Constants.HolographicStyle.Secondary,
tertiaryColor: Constants.HolographicStyle.Tertiary
} : {
primaryColor: primaryColor,
secondaryColor: client.guild.features.includes('ENHANCED_ROLE_COLORS') ? secondaryColor : null

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-on from line 51 - this one also needs to respect allowEnhancedRoleColors, otherwise a stored gradient is restored on a server where the admin disabled the feature.

Suggested change
secondaryColor: client.guild.features.includes('ENHANCED_ROLE_COLORS') ? secondaryColor : null
secondaryColor: enhancedColors ? secondaryColor : null

},
hoist: moduleConf.listRoles,
position: pos,
permissions: '',
Expand Down
38 changes: 38 additions & 0 deletions modules/color-me/migrations/colorme_Role__V1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const {DataTypes} = require('sequelize');

const TABLE = 'colorme_Role';

module.exports = {
// Tables to snapshot before this migration runs (see Backups). Optional but recommended.
tables: [TABLE],

up: async ({context: {queryInterface, sequelize}}) => {
await sequelize.transaction(async (transaction) => {
const description = await queryInterface.describeTable(TABLE).catch(() => ({}));

if (!description.primaryColor && description.color) {
await queryInterface.renameColumn(TABLE, 'color', 'primaryColor', {transaction});
}
if (!description.secondaryColor) {
await queryInterface.addColumn(TABLE, 'secondaryColor', {
type: DataTypes.STRING
}, {transaction});
}
if (!description.holo) {
await queryInterface.addColumn(TABLE, 'holo', {
Comment thread
hfgd123 marked this conversation as resolved.
defaultValue: false,
type: DataTypes.BOOLEAN
}, {transaction});
}
});
},

down: async ({context: {queryInterface, sequelize}}) => {
await sequelize.transaction(async (transaction) => {
const description = await queryInterface.describeTable(TABLE).catch(() => ({}));
if (description.primaryColor && !description.color) await queryInterface.renameColumn(TABLE, 'primaryColor', 'color', {transaction});
if (description.secondaryColor) await queryInterface.removeColumn(TABLE, 'secondaryColor', {transaction});
if (description.holo) await queryInterface.removeColumn(TABLE, 'holo', {transaction});
});
}
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: no trailing newline. (@stylistic/eol-last is off, so this is cosmetic.)

7 changes: 6 additions & 1 deletion modules/color-me/models/Role.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ module.exports = class Role extends Model {
userID: DataTypes.STRING,
roleID: DataTypes.STRING,
name: DataTypes.STRING,
color: DataTypes.STRING,
primaryColor: DataTypes.STRING,
secondaryColor: DataTypes.STRING,
holo: {
type: DataTypes.BOOLEAN,
defaultValue: false
},
timestamp: DataTypes.DATE
}, {
tableName: 'colorme_Role',
Expand Down
Loading
Loading