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

Use string-store for customId creation #10

Open
Amgelo563 opened this issue Nov 9, 2024 · 0 comments
Open

Use string-store for customId creation #10

Amgelo563 opened this issue Nov 9, 2024 · 0 comments
Labels
package: core Addresses the core package package: framework Addresses the framework package priority: medium Medium priority issue status: needs information Lacks information for further progress type: feature request Addition of a new feature

Comments

@Amgelo563
Copy link
Member

The current customId de/serialization logic works through the CustomIdBuilders, which uses indexes to add metadata and user generated data to the string. This works, but it comes with a few caveats, namely:

  • Working with multiple indexes can get messy, specially with non-optional after optional ones.
  • No compression, which can cause problems with Discord's 100 char limit. We throw an error if the limit is reached, but ideally we should compress this.
  • Completely untyped data, as everything comes as strings. Though one can argue that's the user's responsibility, it'd be good if we could at least help with something.

All of these issues can be solved with @sapphire/string-store, which is also made with Discord customId's in mind, unlike other string serialization libraries.

We need to solve one main issue to implement it, though: should it be included in core, or only framework?

Including it in core

Advantages

  • Very easy to implement, since no abstractions are needed.

Disadvantages

  • core is meant to be minimal. Adding a new library to it goes against that idea.
  • We'd be forcing users to use it to serialize customIds. While I do argue that it's probably the best alternative, it's still forcing users into something that they can't avoid.

Only in framework

Advantages

  • Even though framework is also meant to be minimal, it's also meant as a recommended/supported implementation of core, and adding a library that we think is the best way of approaching a problem would go in favor of that.
  • Users aren't forced into using it, since they can just swap the objects that depend it with their own implementations.

Disadvantages

  • We'd need to abstract away the library (mostly unfeasible).
  • How do we approach command customId references? We could simply make the CustomIdBuilder return a string, but then the user won't be able to append any extra data. The only way to export a buildable one would be to make core itself return the store or schema, aka. depending on it. We can't completely remove customId serialization, since sessions depend on that (specifically to extract the ID). I'm currently thinking about using interaction IDs instead, but that'll be tackled in another issue, and this will be later revised.

In essence the current most considered approaches are:

  1. Make core depend on it, and basically replace every method that returns CustomIdBuilder with a schema builder.
  2. Make only framework depend on it, but completely remove every customId related feature (command customId references, session IDs would need to use something else).
@Amgelo563 Amgelo563 added type: feature request Addition of a new feature status: needs information Lacks information for further progress priority: medium Medium priority issue package: core Addresses the core package package: framework Addresses the framework package labels Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core Addresses the core package package: framework Addresses the framework package priority: medium Medium priority issue status: needs information Lacks information for further progress type: feature request Addition of a new feature
Projects
None yet
Development

No branches or pull requests

1 participant