Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Pomelo #1049

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft

Implement Pomelo #1049

wants to merge 16 commits into from

Conversation

Puyodead1
Copy link
Contributor

@Puyodead1 Puyodead1 commented May 5, 2023

The Pomelo project is the code name for Discords new naming system. Pomelo removes the use of discriminators and introduces new, unique usernames (@username) and display names.

The official API docs are still being written, so all changes are based on information the below articles, observing changes in the Discord API, and the partial WIP docs.

  • global_name user property
  • Add Config option (uniqueUsernames)
  • Update user modify route
  • Auto migration

References:

@Puyodead1 Puyodead1 added the Enhancement New feature or request label May 5, 2023
Copy link
Contributor

@erkinalp erkinalp left a comment

Choose a reason for hiding this comment

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

global_name field is the display name. Usernames are going to still use the username field (see discord docs PR https://github.com/discord/discord-api-docs/pull/6130/files/9a162cf08604dc39b10d65f715ed1d87b6b52678#diff-39e6aeaafba5726f3fb409da21a6953dfb679480f09038a0d05f4dd93e89e074)

assets/openapi.json Outdated Show resolved Hide resolved
assets/openapi.json Outdated Show resolved Hide resolved
assets/openapi.json Outdated Show resolved Hide resolved
assets/openapi.json Outdated Show resolved Hide resolved
assets/openapi.json Outdated Show resolved Hide resolved
src/util/entities/User.ts Outdated Show resolved Hide resolved
src/util/schemas/responses/GuildBansResponse.ts Outdated Show resolved Hide resolved
src/util/schemas/responses/GuildWidgetJsonResponse.ts Outdated Show resolved Hide resolved
src/util/schemas/responses/UserRelationsResponse.ts Outdated Show resolved Hide resolved
src/util/util/email/index.ts Outdated Show resolved Hide resolved
@Puyodead1
Copy link
Contributor Author

Puyodead1 commented May 6, 2023

global_name field is the display name. Usernames are going to still use the username field (see discord docs PR https://github.com/discord/discord-api-docs/pull/6130/files/9a162cf08604dc39b10d65f715ed1d87b6b52678#diff-39e6aeaafba5726f3fb409da21a6953dfb679480f09038a0d05f4dd93e89e074)

Not according to the stuff I've read, global name is the handle and username is the display name. huh, ok. I looked at the docs again and I guess I read incorrectly or something?

I also found that docs pr after I wrote this, I'll have to remove the display_name property since it doesn't seem like it's going to be used.

@erkinalp
Copy link
Contributor

erkinalp commented May 6, 2023

/users/@me/pomelo
/users/@me/pomelo-attempt

Those are only going to be used during migration; in spacebar, we might instead want to do an automated migration to unique names.

src/util/entities/User.ts Outdated Show resolved Hide resolved
src/util/entities/User.ts Outdated Show resolved Hide resolved
src/util/entities/User.ts Outdated Show resolved Hide resolved
src/util/entities/User.ts Outdated Show resolved Hide resolved
src/util/config/types/GeneralConfiguration.ts Outdated Show resolved Hide resolved
src/connections/Discord/index.ts Outdated Show resolved Hide resolved
src/api/routes/users/@me/index.ts Outdated Show resolved Hide resolved
src/api/routes/channels/#channel_id/messages/index.ts Outdated Show resolved Hide resolved
src/api/routes/channels/#channel_id/messages/index.ts Outdated Show resolved Hide resolved
src/api/routes/channels/#channel_id/messages/index.ts Outdated Show resolved Hide resolved
src/util/schemas/RelationshipPostSchema.ts Outdated Show resolved Hide resolved
@erkinalp
Copy link
Contributor

from @Puyodead1:

we should also think about what we're going to do about switching back and forth, like if someone has an instance already and then decides to switch to pomelo, and then turns it off again.

I still think this should be allowed, but without full round-tripping compatibility. Discriminators to unique usernames is already a lossy conversion in itself.

@Puyodead1
Copy link
Contributor Author

from @Puyodead1:

we should also think about what we're going to do about switching back and forth, like if someone has an instance already and then decides to switch to pomelo, and then turns it off again.

I still think this should be allowed, but without full round-tripping compatibility. Discriminators to unique usernames is already a lossy conversion in itself.

No it's not? As I said in discord, we can just simply not delete discriminators from the db,and with legacy username (users without one could just keep what they have)

@@ -115,6 +115,7 @@ export const Email: {
const replacements = [
["{instanceName}", instanceName],
["{userUsername}", user.username],
["{userGlobalName}", user.global_name],
Copy link
Member

Choose a reason for hiding this comment

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

this will need to be added to our docs

if (!x.author)
x.author = User.create({
id: "4",
discriminator: "0000",
username: "Spacebar Ghost",
discriminator: uniqueUsernames ? "0" : "0000",
Copy link
Member

Choose a reason for hiding this comment

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

it might be easier to just change the ghost id to be 0 always instead lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants