-
Notifications
You must be signed in to change notification settings - Fork 598
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
replace dmd with dms #3088
base: next
Are you sure you want to change the base?
replace dmd with dms #3088
Conversation
The code itself is fine. I don't know if this has been brought up, but I think there's a small footgun here that users might forget to switch from days to seconds and they'll then be banning people for 7 seconds instead of 7 days... Would it be better to add a |
I don't think that its a big issue, especially because it'll be in the changelog. And a small correction here, its not banning them for 7 seconds, its deleting their messages in the past 7 seconds. So at most developers wouldn't delete as many messages if they didn't read the changelog. I'm not really a fan of implementing a new method, deprecating the old one and then just switching in internally for the next major release under the old name? I don't know, could be fine though? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a few comment issues here, but besides that looks fine. I still don't like swapping it out without at least making a relatively big deal in the changelog. For some people this might trigger a mismatched types error, hopefully that helps soften impact here.
This should always cause a type error anyways unless you're doing a try_into/into thing. But yeah, should be in changelog |
Implicit casting is what I'm thinking of with hardcoded integer values specifically. |
Yeah, since the type is now widened, then integer literals will always typecheck correctly.
I'd even say that the two methods could potentially live side by side, with |
Technically, couldnt this be solved by just making a new DeleteMessageSeconds(u32) wrapper type just like how we have wrapper types for ShardId and other id types? Just wondering out of curiosity |
…erenity-rs#2646) This avoids having to allocate to store fixed length (replaced with normal array) or fixed capacity (replaced with `ArrayVec`) collections as vectors for the purposes of putting them through the `Request` plumbing. Slight behavioral change - before, setting `params` to `Some(vec![])` would still append a question mark to the end of the url. Now, we check if the params array `is_empty` instead of `is_some`, so the question mark won't be appended if the params list is empty. Co-authored-by: Michael Krasnitski <[email protected]>
This trades a heap allocation for messages sent along with thread creation for `Message`'s inline size dropping from 1176 bytes to 760 bytes,
…l models (serenity-rs#2656) This shrinks type sizes by a lot; however, it makes the user experience slightly different: - `FixedString` must be converted to String with `.into()` or `.into_string()` before it can be pushed to, but dereferences to `&str` as is. - `FixedArray` must be converted to `Vec` with `.into()` or `.into_vec()` before it can be pushed to, but dereferences to `&[T]` as is. The crate of these types is currently a Git dependency, but this is fine for the `next` branch. It needs some basic testing, which Serenity is perfect for, before a release will be made to crates.io.
…s push over the limit (serenity-rs#2660)
…enity-rs#2668) This commit: - switches from `u64` to `i64` in `CreateCommandOption::min_int_value` and `CreateCommandOption::max_int_value` to accommodate negative integers in Discord's integer range (between -2^53 and 2^53). Values outside this range will cause Discord's API to return an error. - switches from `i32` to `i64` in `CreateCommandOption::add_int_choice` and `CreateCommandOption::add_int_choice_localized` to accommodate Discord's complete integer range (between -2^53 and 2^53). Values outside this range will cause Discord's API to return an error.
This cache was just duplicating information already present in `Guild::members` and therefore should be removed. This saves around 700 MBs for my bot (pre-`FixedString`). This has to refactor `utils::content_safe` to always take a `Guild` instead of`Cache`, but in practice it was mostly pulling from the guild cache anyway and this means it is more likely to respect nicknames and other information, while losing the ability to clean mentions from DMs, which do not matter.
`Embed::fields` previously had to stay as a `Vec` due to `CreateEmbed` wrapping around it, but by implementing `Serialize` manually we can overwrite the `Embed::fields` with a normal `Vec`, for a small performance hit on serialization while saving some space for all stored `Embed`s.
Simply missed these when finding and replacing.
This uses the `bool_to_bitflags` macro to remove boolean (and optional boolean) fields from structs and pack them into a bitflags invocation, so a struct with many bools will only use one or two bytes, instead of a byte per bool as is. This requires using getters and setters for the boolean fields, which changes user experience and is hard to document, which is a significant downside, but is such a nice change and will just become more and more efficient as time goes on.
…rs#2681) This swaps fields that store `Option<Int>` for `Option<NonMaxInt>` where the maximum value would be ludicrous. Since `nonmax` uses `NonZero` internally, this gives us niche optimisations, so model sizes can drop some more. I have had to include a workaround for [serenity-rs#17] in `optional_string` by making my own `TryFrom<u64>`, so that should be removable once that issue is fixed. [serenity-rs#17]: LPGhatguy/nonmax#17
A couple of clippy bugs have been fixed and I have shrunk model sizes enough to make `clippy::large_enum_variant` go away.
A discord bot library should not be using the tools reserved for low level OS interaction/data structure libraries.
Discord seems to internally default Ids to 0, which is a bug whenever exposed, but this makes ID parsing more resilient. I also took the liberty to remove the `From<NonZero*>` implementations, to prevent future headaches, as it was impossible to not break public API as we exposed `NonZero` in `*Id::parse`.
…nity-rs#2694) This, 1. shrinks the size of Request, when copied around, as it doesn't have to store the max capacity at all times 2. shrinks llvm-lines (compile time metric) for my bot in debug from `1,153,519` to `1,131,480` as no monomorphisation has to be performed for `MAX_PARAMS`.
This allows it to imported via `poise::serenity_prelude` which is often `use`'d as `serenity`.
This was missing on FullEvent but was present within the event itself.
Values are getting quite close to the u8 limit (193), so let's preemptively bump this to avoid future breaking changes.
Moves the feature-gated collector methods on `UserId`, `MessageId`, etc. into four traits: - `CollectMessages` - `CollectReactions` - `CollectModalInteractions` - `CollectComponentInteractions` This also moves the quick modal machinery into the collector module and defines a `QuickModal` trait. This fully removes any collector feature gates from the model types.
This file was removed from the module tree but never actually deleted.
Fixes CI failure
…rs#3075) Similar to message URLs, Discord also provides URLs for guild channels. Additionally, the function is added as an alternative for parsing a `Channel` from a string. Private channels are not affected by this change.
The `Deserialize` implementation neglects to add the `Bot ` prefix to the string when it is deserialised. This adds `TryFrom` implementations for `&str` and `String` and tells serde to deserialise `Token` using the `TryFrom<&str>` implementation, which will prepend the `Bot ` prefix. Fixes serenity-rs#3085
Co-authored-by: Niko <[email protected]>
Co-authored-by: Niko <[email protected]>
Co-authored-by: Niko <[email protected]>
Co-authored-by: Niko <[email protected]>
Co-authored-by: Niko <[email protected]>
4f21538
to
3908461
Compare
I'd say this is fine, but I'll leave it to someone else to make the final decision on if this is okay (as it may confuse bot developers and lead them to delete messages far less than they intended to) |
Take 2 on the delete_message_seconds PR. This also removes the pre-validation code as discussed in #lib-development and by extension currently removes the broken example in src/model/error.rs