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

feat!: rework model enums #2045

Draft
wants to merge 13 commits into
base: next
Choose a base branch
from
Draft

Conversation

zeylahellyer
Copy link
Member

@zeylahellyer zeylahellyer commented Jan 8, 2023

Description is a work in progress

Rework model enums to more cleanly map to raw values and make it so that variants unknown to the library don't feel like "second class" citizens to be avoided.

This PR is possibly aimed for Twilight 0.16, but perhaps more likely 0.17.

Motivation

I was reviewing and merging PRs and saw PR #1922, which is a PR that changes the Guild::afk_timeout type from an integer to an enum. A review comment suggested using associated constants for AFK timeout values instead of enum variants. I tended to agree, since AFK timeout values are somewhat "arbitrary", in that rather than being incremental values (1, 2, 3, ...) the accepted values were a range of indeterminate values (60, or "one minute"; 300, or "five minutes"; 900, or "fiften minutes"; etc.). Reviewers on the PR agreed this direction looked cleaner and nicer to use. It looked like this:

`AfkTimeout` implementation
#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
pub struct AfkTimeout(u16);

impl AfkTimeout {
    /// AFK timeout of one minute.
    pub const ONE_MINUTE: Self = Self(60);

    /// AFK timeout of five minutes.
    pub const FIVE_MINUTES: Self = Self(300);

    /// AFK timeout of fifteen minutes.
    pub const FIFTEEN_MINUTES: Self = Self(900);

    /// AFK timeout of thirty minutes.
    pub const THIRTY_MINUTES: Self = Self(1800);

    /// AFK timeout of one hour.
    pub const ONE_HOUR: Self = Self(3600);

    /// Retrieve the duration of the AFK timeout in seconds.
    ///
    /// # Examples
    ///
    /// ```
    /// use twilight_model::guild::AfkTimeout;
    ///
    /// assert_eq!(60, AfkTimeout::ONE_MINUTE.get());
    /// ```
    pub const fn get(self) -> u16 {
        self.0
    }
}

impl From<u16> for AfkTimeout {
    fn from(value: u16) -> Self {
        Self(value)
    }
}

impl From<AfkTimeout> for Duration {
    fn from(value: AfkTimeout) -> Self {
        Self::from_secs(u64::from(value.get()))
    }
}

impl PartialEq<u16> for AfkTimeout {
    fn eq(&self, other: &u16) -> bool {
        self.get() == *other
    }
}

impl PartialEq<AfkTimeout> for u16 {
    fn eq(&self, other: &AfkTimeout) -> bool {
        *self == other.get()
    }
}

Later, I created PR #2018, which added a module with public constants (not on a type) of known valid scopes:

pub const ACTIVITIES_READ: &str = "activities.read";

pub const ACTIVITIES_WRITE: &str = "activities.write";

It was again suggested to use a type with associated constants since it felt like a good design in the AFK timeout PR. And, again, I agreed. However, doing so for scopes wasn't as clean: scopes are strings in the underlying type, while the AFK timeout struct uses an integer. You need to have owned strings for deserializing from serde contexts, but borrowed (and static) values for associated constants. Cows aren't a solution, because they don't work in all constant contexts. Thus, I made Scope be a wrapper over some bytes so it could be used in associated constants and (de)serialized from/to serde contexts.

Now that I had this working I saw that this was a lot of work just to make a Scope type. We have a lot of enums in the twilight-model crate that are similar to AfkTimeout and Scope, in that they have "arbitrary values". This presented me with a question: what if we extend this to all enums like it? Other enums similar to these are composed of types such as AutoArchiveDuration, GuildFeature, and IntegrationExpireBehavior. AutoArchiveDuration is an enum with variants of Hour, Day, ThreeDays, Week, and an Unknown variant for values that aren't registered with Twilight. GuildFeature is an enum with variants for known features, such as AnimatedBanner, AutoModeration, and Discoverable, and again with an Unknown variant. IntegrationExpireBehavior is an enum that declares what should happen when an integration expires, and its variants are Kick, RemoveRole, and Unknown.

These are three enums that feel right as being classified as "arbitrary values". AutoArchiveDuration isn't incremental and contains values that map more to what makes sense for Discord's UI; GuildFeature is a growing list of strings that are entirely arbitrary, and is added to and removed from often in Discord's API; lastly, IntegrationExpireBehavior is a type with variants where the mapped values can't be made sense of on their own: no one would guess that the listed order of variants above was in the wrong order, because Kick is actually value 1 while RemoveRole is actually value 0.

The point can be made that this is true of every enum we have mapping to a value. For example, ChannelType has many variants that are mostly incremental, with value 0 being GuildText and value 10 being AnnouncementThread. These values, although nonsensical on their own, are important. They're added to as the API and Discord's types of channels grows. Twilight certainly will never have all of them at all times. The raw integers are the value, not the enum variant name.

As a result of this thinking, this PR modifies every enum without data (such as Component) to use this scheme. It makes every type consistent; it makes one think about the value itself and not the abstraction of the value; it's clean to use; and "unknown values" no longer feel like a side-effect, they're a feature.

Definition Examples

Explicit Content Filter (Integer)

Before
#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
#[non_exhaustive]
#[serde(from = "u8", into = "u8")]
pub enum ExplicitContentFilter {
    None,
    MembersWithoutRole,
    AllMembers,
    /// Variant value is unknown to the library.
    Unknown(u8),
}

impl From<u8> for ExplicitContentFilter {
    fn from(value: u8) -> Self {
        match value {
            0 => ExplicitContentFilter::None,
            1 => ExplicitContentFilter::MembersWithoutRole,
            2 => ExplicitContentFilter::AllMembers,
            unknown => ExplicitContentFilter::Unknown(unknown),
        }
    }
}

impl From<ExplicitContentFilter> for u8 {
    fn from(value: ExplicitContentFilter) -> Self {
        match value {
            ExplicitContentFilter::None => 0,
            ExplicitContentFilter::MembersWithoutRole => 1,
            ExplicitContentFilter::AllMembers => 2,
            ExplicitContentFilter::Unknown(unknown) => unknown,
        }
    }
}
After
#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
pub struct ExplicitContentFilter(u8);

impl ExplicitContentFilter {
    pub const NONE: Self = Self::new(0);
    pub const MEMBERS_WITHOUT_ROLE: Self = Self::new(1);
    pub const ALL_MEMBERS: Self = Self::new(2);

    /// Create a new explicit content filter from a dynamic value.
    ///
    /// The provided value isn't validated. Known valid values are associated
    /// constants such as [`NONE`][`Self::NONE`].
    pub const fn new(explicit_content_filter: u8) -> Self {
        Self(explicit_content_filter)
    }

    /// Retrieve the value of the explicit content filter.
    ///
    /// # Examples
    ///
    /// ```
    /// use twilight_model::guild::ExplicitContentFilter;
    ///
    /// assert_eq!(2, ExplicitContentFilter::ALL_MEMBERS.get());
    /// ```
    pub const fn get(&self) -> u8 {
        self.0
    }
}

impl From<u8> for ExplicitContentFilter {
    fn from(value: u8) -> Self {
        Self(value)
    }
}

impl From<ExplicitContentFilter> for u8 {
    fn from(value: ExplicitContentFilter) -> Self {
        value.get()
    }
}

Guild Feature (String)

Before
#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
#[non_exhaustive]
#[serde(from = "String", into = "Cow<'static, str>")]
pub enum GuildFeature {
    /// Has access to set an animated guild banner image.
    AnimatedBanner,
    /// Has access to set an animated guild icon.
    AnimatedIcon,
    /// Has set up auto moderation rules.
    AutoModeration,
    /// Has access to set a guild banner image.
    Banner,
    /// Has access to use commerce features (create store channels).
    #[deprecated]
    Commerce,
    /// Can enable welcome screen, membership screening, stage channels,
    /// discovery, and receives community updates.
    Community,
    /// Guild has been set as a support server on the App Directory.
    DeveloperSupportServer,
    /// Is able to be discovered in the directory.
    Discoverable,
    /// Is able to be featured in the directory.
    Featurable,
    /// Invites have been paused, this prevents new users from joining.
    InvitesDisabled,
    /// Has access to set an invite splash background.
    InviteSplash,
    /// Has enabled membership screening.
    MemberVerificationGateEnabled,
    /// Has enabled monetization.
    MonetizationEnabled,
    /// Has increased custom sticker slots.
    MoreStickers,
    /// Has access to create news channels.
    News,
    /// Is partnered.
    Partnered,
    /// Can be previewed before joining via membership screening or the directory.
    PreviewEnabled,
    /// Has access to create private threads.
    PrivateThreads,
    /// Is able to set role icons.
    RoleIcons,
    /// Has enabled ticketed events.
    TicketedEventsEnabled,
    /// Has access to set a vanity URL.
    VanityUrl,
    /// Is verified.
    Verified,
    /// Has access to set 384kps bitrate in voice (previously VIP voice servers).
    VipRegions,
    /// Has enabled the welcome screen.
    WelcomeScreenEnabled,
    /// Variant value is unknown to the library.
    Unknown(String),
}

impl From<GuildFeature> for Cow<'static, str> {
    fn from(value: GuildFeature) -> Self {
        match value {
            GuildFeature::AnimatedBanner => "ANIMATED_BANNER".into(),
            GuildFeature::AnimatedIcon => "ANIMATED_ICON".into(),
            GuildFeature::AutoModeration => "AUTO_MODERATION".into(),
            GuildFeature::Banner => "BANNER".into(),
            GuildFeature::Commerce => "COMMERCE".into(),
            GuildFeature::Community => "COMMUNITY".into(),
            GuildFeature::DeveloperSupportServer => "DEVELOPER_SUPPORT_SERVER".into(),
            GuildFeature::Discoverable => "DISCOVERABLE".into(),
            GuildFeature::Featurable => "FEATURABLE".into(),
            GuildFeature::InvitesDisabled => "INVITES_DISABLED".into(),
            GuildFeature::InviteSplash => "INVITE_SPLASH".into(),
            GuildFeature::MemberVerificationGateEnabled => {
                "MEMBER_VERIFICATION_GATE_ENABLED".into()
            }
            GuildFeature::MonetizationEnabled => "MONETIZATION_ENABLED".into(),
            GuildFeature::MoreStickers => "MORE_STICKERS".into(),
            GuildFeature::News => "NEWS".into(),
            GuildFeature::Partnered => "PARTNERED".into(),
            GuildFeature::PreviewEnabled => "PREVIEW_ENABLED".into(),
            GuildFeature::PrivateThreads => "PRIVATE_THREADS".into(),
            GuildFeature::RoleIcons => "ROLE_ICONS".into(),
            GuildFeature::TicketedEventsEnabled => "TICKETED_EVENTS_ENABLED".into(),
            GuildFeature::VanityUrl => "VANITY_URL".into(),
            GuildFeature::Verified => "VERIFIED".into(),
            GuildFeature::VipRegions => "VIP_REGIONS".into(),
            GuildFeature::WelcomeScreenEnabled => "WELCOME_SCREEN_ENABLED".into(),
            GuildFeature::Unknown(unknown) => unknown.into(),
        }
    }
}

impl From<String> for GuildFeature {
    fn from(value: String) -> Self {
        match value.as_str() {
            "ANIMATED_BANNER" => Self::AnimatedBanner,
            "ANIMATED_ICON" => Self::AnimatedIcon,
            "AUTO_MODERATION" => Self::AutoModeration,
            "BANNER" => Self::Banner,
            "COMMERCE" => Self::Commerce,
            "COMMUNITY" => Self::Community,
            "DEVELOPER_SUPPORT_SERVER" => Self::DeveloperSupportServer,
            "DISCOVERABLE" => Self::Discoverable,
            "FEATURABLE" => Self::Featurable,
            "INVITES_DISABLED" => Self::InvitesDisabled,
            "INVITE_SPLASH" => Self::InviteSplash,
            "MEMBER_VERIFICATION_GATE_ENABLED" => Self::MemberVerificationGateEnabled,
            "MONETIZATION_ENABLED" => Self::MonetizationEnabled,
            "MORE_STICKERS" => Self::MoreStickers,
            "NEWS" => Self::News,
            "PARTNERED" => Self::Partnered,
            "PREVIEW_ENABLED" => Self::PreviewEnabled,
            "PRIVATE_THREADS" => Self::PrivateThreads,
            "ROLE_ICONS" => Self::RoleIcons,
            "TICKETED_EVENTS_ENABLED" => Self::TicketedEventsEnabled,
            "VANITY_URL" => Self::VanityUrl,
            "VERIFIED" => Self::Verified,
            "VIP_REGIONS" => Self::VipRegions,
            "WELCOME_SCREEN_ENABLED" => Self::WelcomeScreenEnabled,
            _ => Self::Unknown(value),
        }
    }
}
After
#[derive(Clone, Copy, Deserialize, Eq, Hash, PartialEq, Serialize)]
pub struct GuildFeature(KnownString<64>);

impl GuildFeature {
    /// Has access to set an animated guild banner image.
    pub const ANIMATED_BANNER: Self = Self::from_bytes(b"ANIMATED_BANNER");

    /// Has access to set an animated guild icon.
    pub const ANIMATED_ICON: Self = Self::from_bytes(b"ANIMATED_ICON");

    /// Has set up auto moderation rules.
    pub const AUTO_MODERATION: Self = Self::from_bytes(b"AUTO_MODERATION");

    /// Has access to set a guild banner image.
    pub const BANNER: Self = Self::from_bytes(b"BANNER");

    /// Has access to use commerce features (create store channels).
    #[deprecated]
    pub const COMMERCE: Self = Self::from_bytes(b"COMMERCE");

    /// Can enable welcome screen, membership screening, stage channels,
    /// discovery, and receives community updates.
    pub const COMMUNITY: Self = Self::from_bytes(b"COMMUNITY");

    /// Guild has been set as a support server on the App Directory.
    pub const DEVELOPER_SUPPORT_SERVER: Self = Self::from_bytes(b"DEVELOPER_SUPPORT_SERVER");

    /// Is able to be discovered in the directory.
    pub const DISCOVERABLE: Self = Self::from_bytes(b"DISCOVERABLE");

    /// Is able to be featured in the directory.
    pub const FEATURABLE: Self = Self::from_bytes(b"FEATURABLE");

    /// Invites have been paused, this prevents new users from joining.
    pub const INVITES_DISABLED: Self = Self::from_bytes(b"INVITES_DISABLED");

    /// Has access to set an invite splash background.
    pub const INVITE_SPLASH: Self = Self::from_bytes(b"INVITE_SPLASH");

    /// Has enabled membership screening.
    pub const MEMBER_VERIFICATION_GATE_ENABLED: Self =
        Self::from_bytes(b"MEMBER_VERIFICATION_GATE_ENABLED");

    /// Has enabled monetization.
    pub const MONETIZATION_ENABLED: Self = Self::from_bytes(b"MONETIZATION_ENABLED");

    /// Has increased custom sticker slots.
    pub const MORE_STICKERS: Self = Self::from_bytes(b"MORE_STICKERS");

    /// Has access to create news channels.
    pub const NEWS: Self = Self::from_bytes(b"NEWS");

    /// Is partnered.
    pub const PARTNERED: Self = Self::from_bytes(b"PARTNERED");

    /// Can be previewed before joining via membership screening or the
    /// directory.
    pub const PREVIEW_ENABLED: Self = Self::from_bytes(b"PREVIEW_ENABLED");

    /// Has access to create private threads.
    pub const PRIVATE_THREADS: Self = Self::from_bytes(b"PRIVATE_THREADS");

    /// Is able to set role icons.
    pub const ROLE_ICONS: Self = Self::from_bytes(b"ROLE_ICONS");

    /// Has enabled ticketed events.
    pub const TICKETED_EVENTS_ENABLED: Self = Self::from_bytes(b"TICKETED_EVENTS_ENABLED");

    /// Has access to set a vanity URL.
    pub const VANITY_URL: Self = Self::from_bytes(b"VANITY_URL");

    /// Is verified.
    pub const VERIFIED: Self = Self::from_bytes(b"VERIFIED");

    /// Has access to set 384kps bitrate in voice (previously VIP voice
    /// servers).
    pub const VIP_REGIONS: Self = Self::from_bytes(b"VIP_REGIONS");

    /// Has enabled the welcome screen.
    pub const WELCOME_SCREEN_ENABLED: Self = Self::from_bytes(b"WELCOME_SCREEN_ENABLED");

    /// Create a guild feature from a dynamic value.
    ///
    /// The provided guild feature must be 64 bytes or smaller.
    pub fn new(guild_feature: &str) -> Option<Self> {
        KnownString::from_str(guild_feature).map(Self)
    }

    /// Get the value of the guild feature.
    ///
    /// # Panics
    ///
    /// Panics if the guild feature isn't valid UTF-8.
    pub fn get(&self) -> &str {
        self.0.get()
    }

    /// Create a guild feature from a set of bytes.
    const fn from_bytes(input: &[u8]) -> Self {
        Self(KnownString::from_bytes(input))
    }
}

impl AsRef<str> for GuildFeature {
    fn as_ref(&self) -> &str {
        self.get()
    }
}

impl Debug for GuildFeature {
    fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
        f.write_str(self.get())
    }
}

impl Deref for GuildFeature {
    type Target = str;

    fn deref(&self) -> &Self::Target {
        self.get()
    }
}

impl FromStr for GuildFeature {
    type Err = ();

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        Self::try_from(s)
    }
}

impl ToString for GuildFeature {
    fn to_string(&self) -> String {
        KnownString::to_string(&self.0)
    }
}

impl TryFrom<&str> for GuildFeature {
    type Error = ();

    fn try_from(value: &str) -> Result<Self, Self::Error> {
        Self::new(value).ok_or(())
    }
}

Use Case Examples

Todo

PR Todo

  • Add description to PR
  • Get CI working?
  • Resolve 2 todo!() comments; not difficult, just didn't get to them
  • Debug formatting types only prints the raw value
  • Document known_string more better
  • Add test suite for known_string
  • Add definition and use case examples to PR
  • Break this PR down into several PRs
    • name methods on remaining model types (these exist on some already, and was extended to all)
    • New/refactored tests for types
    • New documentation that was surfaced
    • Proc macro to generate methods (such as name) and trait implementations (such as Debug, FromStr)
  • Implement more traits on these types
  • Scope impact on applications
  • Scope benefits and drawbacks of PR
  • Changelog entry

@zeylahellyer zeylahellyer added t-feature Addition of a new feature c-http Affects the http crate c-cache Affects the cache crate c-model Affects the model crate c-gateway Affects the gateway crate m-breaking change Breaks the public API. c-standby Affects the standby crate c-all Affects all crates or the project as a whole c-util Affects the util crate t-refactor Refactors APIs or code. w-do-not-merge PR is blocked or deferred w-unapproved Proposal for change has *not* been approved by @twilight-rs/core. w-needs-testing PR needs to be tested. c-validate Affects the validate crate w-needs-more-docs Needs more documentation before being worked on. t-perf Improves performace c-book Affects the book labels Jan 8, 2023
@github-actions github-actions bot removed the c-book Affects the book label Jan 8, 2023
@vilgotf

This comment was marked as resolved.

@github-actions github-actions bot added the c-book Affects the book label Jan 8, 2023
zeylahellyer added a commit that referenced this pull request Jan 15, 2023
Adds a typed Locale struct with associated constants for all known
values, replacing the Strings used in the codebase. The Locale type has
two methods of note: `english_name` and `native_name`.

These values are taken from the Discord documentation:
<https://discord.com/developers/docs/reference#locales>

This PR is meant to show how the new raw value representation introduced
in PR #2045 looks when "typing" a value. This was previously left to the
user to know what localizations might exist and to ensure a typo isn't
made. This `Locale` type is still functionally a string in all but name
(literally, as it implements the same traits and derefs in the same way
String does), but it contains methods and the known values registered by
Twilight as "sugar" on top of it.
@zeylahellyer zeylahellyer linked an issue Jan 19, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-all Affects all crates or the project as a whole c-book Affects the book c-cache Affects the cache crate c-gateway Affects the gateway crate c-http Affects the http crate c-model Affects the model crate c-standby Affects the standby crate c-util Affects the util crate c-validate Affects the validate crate m-breaking change Breaks the public API. t-feature Addition of a new feature t-perf Improves performace t-refactor Refactors APIs or code. w-do-not-merge PR is blocked or deferred w-needs-more-docs Needs more documentation before being worked on. w-needs-testing PR needs to be tested. w-unapproved Proposal for change has *not* been approved by @twilight-rs/core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Handling unknown enum variants
3 participants