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.

Sign up for GitHub

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

Jump to bottom

Implements changes to allow developers more control about event handling #2854

Open
wants to merge 122 commits into
base: next
Choose a base branch
from
Open

Implements changes to allow developers more control about event handling #2854

wants to merge 122 commits into from

Conversation

TheCataliasTNT2k
Copy link

Adds Framework::dispatch_automatically
Makes client::dispatch::update_cache_with_event public, so it can be called manually by developers

@github-actions github-actions bot added framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate client Related to the `client` module. labels Apr 28, 2024
TheCataliasTNT2k added a commit to RuDrocsid/poise that referenced this pull request Apr 28, 2024
@TheCataliasTNT2k
Updates framework::Framework to match serenity-rs/serenity#2854
48b8385
@TheCataliasTNT2k TheCataliasTNT2k mentioned this pull request Apr 28, 2024
Open
TheCataliasTNT2k added a commit to RuDrocsid/poise that referenced this pull request Apr 28, 2024
@TheCataliasTNT2k
Updates framework::Framework to match serenity-rs/serenity#2854
5accb8c
TheCataliasTNT2k added a commit to RuDrocsid/poise that referenced this pull request Apr 28, 2024
@TheCataliasTNT2k
Updates framework::Framework to match serenity-rs/serenity#2854
746b5c4
@TheCataliasTNT2k TheCataliasTNT2k force-pushed the framework_manual_dispatch branch from a52e915 to 4040274 Compare April 29, 2024 16:36
GnomedDev
GnomedDev requested changes Apr 30, 2024
View reviewed changes
Copy link
Member

@GnomedDev GnomedDev left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I dislike update_cache_with_event becoming public, as it's quite internal. If you could make a nicer API for this that doesn't require making internal modules public, that would resolve this.

@TheCataliasTNT2k
Copy link
Author

TheCataliasTNT2k commented Apr 30, 2024 •
edited
Loading

The only thing in this method I really care about, is to get the returned values.
I do not really want to copy this function to get the FullEvent objects to use them in my library. If this can be achieved in any other way (while not losing the cache functionality) I am happy.

@cheesycod
Copy link
Contributor

This would be useful for me as well

@GnomedDev GnomedDev force-pushed the next branch 3 times, most recently from 7ad183d to 52f2822 Compare May 14, 2024 20:34
@GnomedDev GnomedDev force-pushed the next branch 3 times, most recently from 8d52f25 to 68e5e62 Compare May 28, 2024 14:58
GnomedDev and others added 14 commits June 9, 2024 22:33
@GnomedDev @mkrasnitski
Remove temporary Vec usage in the http module (serenity-rs#2624, s…
062e685 
…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]>
@GnomedDev
Remove deprecated items (serenity-rs#2645)
007c360
@GnomedDev
Remove more deprecated items (serenity-rs#2651)
8df3f63
@GnomedDev
Remove *_arc methods (serenity-rs#2654)
651380e 
These are unnecessary. Accepting `impl Into<Arc<T>>` allows passing either `T` or `Arc<T>`.
@GnomedDev
Clean up ShardManager/ShardQueuer/ShardRunner (serenity-rs#2653)
c564016
@GnomedDev
Put Message::thread behind a Box (serenity-rs#2658)
be79bdb 
This trades a heap allocation for messages sent along with thread
creation for `Message`'s inline size dropping from 1176 bytes to 760
bytes,
@GnomedDev
Replace Vec and String with FixedArray and FixedString for al…
1526f9b 
…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.
@GnomedDev
Fix truncation when using FixedString<u8> where multibyte character…
2db8487 
…s push over the limit (serenity-rs#2660)
@Kerollmops @GnomedDev
Encode timestamps in RFC3339 (serenity-rs#2665)
f20e8fc 
Fixes serenity-rs#2664
@ARandomDev99 @GnomedDev
Switch to i64 in methods associated with CreateCommandOption (ser…
686c5dc 
…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.
@GnomedDev
Get rid of the duplicate users cache (serenity-rs#2662)
b59131f 
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.
@GnomedDev
Change Embed::fields to use FixedArray (serenity-rs#2674)
916dd12 
`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.
@GnomedDev
Use FixedArray and FixedString in model enums (serenity-rs#2675)
eb0a598 
Simply missed these when finding and replacing.
@GnomedDev
Bitpack boolean fields using fancy proc macro (serenity-rs#2673)
5589be3 
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.
mkrasnitski and others added 11 commits June 9, 2024 23:28
@mkrasnitski @GnomedDev
Restore match arm for Error::Gateway variant (serenity-rs#2847)
b213438 
This was mistakenly removed in serenity-rs#2278 and wasn't caught because of the
wildcard pattern in the match.
@GnomedDev
Remove multiple event handler support (serenity-rs#2849)
3d50a65
@TheCataliasTNT2k @GnomedDev
Use RwLock for active collectors collection (serenity-rs#2851)
6406655
@jamesbt365 @GnomedDev
Provide old presence data from the cache on presence update (serenity…
8edab28 
…-rs#2852)
@TheCataliasTNT2k @GnomedDev
Allow filtering events before they are dispatched (serenity-rs#2853)
fe0f087
@jamesbt365 @GnomedDev
Remove permission checks (serenity-rs#2855)
f01fd2b 
Users should realistically be checking the permissions themselves, or
handling the HTTP error from Discord.

This removes any cases where permission checking inside the library is
broken because of Discord's changes or due to oversights.

This also changes the documentation on the prune functionality, this was
recently changed to also require `MANAGE_GUILDS` as well as
`KICK_MEMBERS`.
@GnomedDev
Implement ToArrayString for Mention (serenity-rs#2857)
74659fc
@cheesycod @GnomedDev
Add PartialGuild::greater_member_hierarchy + member_highest_role (ser…
776185b 
…enity-rs#2856)
@GnomedDev
Remove recently deprecated items
e855379
@GnomedDev
Update bool_to_bitflags to fix compilation without unstable
7f18f11
@jamesbt365 @GnomedDev
Remove RUSTFLAGS from .cargo/config.toml (serenity-rs#2875)
3e03891 
This isn't needed anymore because simd-json has been removed on `next`.
@TheCataliasTNT2k
Copy link
Author

TheCataliasTNT2k commented Jun 10, 2024 •
edited
Loading

I can not really create a better API, because I need to update the cache.
Do you have any suggestions?
(I will resolve the conflicts when all changes are approved.)

I dislike update_cache_with_event becoming public, as it's quite internal. If you could make a nicer API for this that doesn't require making internal modules public, that would resolve this.

Already did the rebase (or rather just moved my commits to the new HEAD).

@TheCataliasTNT2k
Adds Framework::dispatch_automatically
bed49b1 
Makes client::dispatch::update_cache_with_event public, so it can be called manually by developers
@TheCataliasTNT2k TheCataliasTNT2k force-pushed the framework_manual_dispatch branch from 4040274 to bed49b1 Compare June 10, 2024 21:17
TheCataliasTNT2k added a commit to RuDrocsid/poise that referenced this pull request Jun 10, 2024
@TheCataliasTNT2k
Updates framework::Framework to match serenity-rs/serenity#2854
56d4903
TheCataliasTNT2k added a commit to RuDrocsid/poise that referenced this pull request Jun 10, 2024
@TheCataliasTNT2k
Updates framework::Framework to match serenity-rs/serenity#2854
72e70ce
@GnomedDev GnomedDev force-pushed the next branch 2 times, most recently from c01e57d to 842373b Compare June 22, 2024 20:38
@cheesycod
Copy link
Contributor

Any updates on this?

@TheCataliasTNT2k
Copy link
Author

I don't think so 🤷
I am just using a fork which has everything I need, which is rejected by the maintainers.

@GnomedDev GnomedDev force-pushed the next branch 3 times, most recently from 9296017 to efb5820 Compare November 15, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GnomedDev GnomedDev GnomedDev requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
client Related to the `client` module. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.