Skip to content

Commit

Permalink
Bitpack boolean fields using fancy proc macro (#2673)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
GnomedDev committed Mar 11, 2024
1 parent 3c684a2 commit 8fca50e
Show file tree
Hide file tree
Showing 29 changed files with 259 additions and 306 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ base64 = { version = "0.21.5" }
secrecy = { version = "0.8.0", features = ["serde"] }
arrayvec = { version = "0.7.4", features = ["serde"] }
small-fixed-array = { git = "https://github.com/GnomedDev/small-fixed-array", features = ["serde", "log_using_tracing"] }
bool_to_bitflags = { git = "https://github.com/GnomedDev/bool-to-bitflags", version = "0.1.0" }
# Optional dependencies
fxhash = { version = "0.2.1", optional = true }
simd-json = { version = "0.13.4", optional = true }
Expand Down Expand Up @@ -125,7 +126,7 @@ simd_json = ["simd-json", "typesize?/simd_json"]
# Enables temporary caching in functions that retrieve data via the HTTP API.
temp_cache = ["cache", "mini-moka", "typesize?/mini_moka"]

typesize = ["dep:typesize", "small-fixed-array/typesize"]
typesize = ["dep:typesize", "small-fixed-array/typesize", "bool_to_bitflags/typesize"]

# Removed feature (https://github.com/serenity-rs/serenity/pull/2246)
absolute_ratelimits = []
Expand Down
4 changes: 2 additions & 2 deletions examples/e05_command_framework/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,10 @@ async fn say(ctx: &Context, msg: &Message, mut args: Args) -> CommandResult {
if let Some(guild) = msg.guild(&ctx.cache) {
// By default roles, users, and channel mentions are cleaned.
let settings = ContentSafeOptions::default()
// We do not want to clean channal mentions as they do not ping users.
// We do not want to clean channel mentions as they do not ping users.
.clean_channel(false);

x = content_safe(&guild, x, &settings, &msg.mentions);
x = content_safe(&guild, x, settings, &msg.mentions);
}

msg.channel_id.say(&ctx.http, x).await?;
Expand Down
7 changes: 3 additions & 4 deletions src/builder/create_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ impl CreateCommandOption {
name_localizations: None,
description: description.into().into(),
description_localizations: None,
required: false,
autocomplete: false,
__generated_flags: CommandOptionGeneratedFlags::empty(),
min_value: None,
max_value: None,
min_length: None,
Expand Down Expand Up @@ -104,7 +103,7 @@ impl CreateCommandOption {
///
/// **Note**: This defaults to `false`.
pub fn required(mut self, required: bool) -> Self {
self.0.required = required;
self.0.set_required(required);
self
}

Expand Down Expand Up @@ -203,7 +202,7 @@ impl CreateCommandOption {
/// - May not be set to `true` if `choices` are set
/// - Options using `autocomplete` are not confined to only use given choices
pub fn set_autocomplete(mut self, value: bool) -> Self {
self.0.autocomplete = value;
self.0.set_autocomplete(value);
self
}

Expand Down
4 changes: 2 additions & 2 deletions src/builder/edit_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ impl<'a> EditRole<'a> {
/// Creates a new builder with the values of the given [`Role`].
pub fn from_role(role: &Role) -> Self {
EditRole {
hoist: Some(role.hoist),
mentionable: Some(role.mentionable),
hoist: Some(role.hoist()),
mentionable: Some(role.mentionable()),
name: Some(role.name.clone()),
permissions: Some(role.permissions.bits()),
position: Some(role.position),
Expand Down
28 changes: 15 additions & 13 deletions src/cache/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use crate::model::event::{
VoiceStateUpdateEvent,
};
use crate::model::gateway::ShardInfo;
use crate::model::guild::{Guild, GuildMemberFlags, Member, Role};
use crate::model::guild::{Guild, GuildMemberFlags, Member, MemberGeneratedFlags, Role};
use crate::model::id::ShardId;
use crate::model::user::{CurrentUser, OnlineStatus};
use crate::model::voice::VoiceState;
Expand Down Expand Up @@ -186,36 +186,40 @@ impl CacheUpdate for GuildMemberUpdateEvent {
member.nick.clone_from(&self.nick);
member.roles.clone_from(&self.roles);
member.user.clone_from(&self.user);
member.pending.clone_from(&self.pending);
member.premium_since.clone_from(&self.premium_since);
member.deaf.clone_from(&self.deaf);
member.mute.clone_from(&self.mute);
member.avatar.clone_from(&self.avatar);
member.communication_disabled_until.clone_from(&self.communication_disabled_until);
member.unusual_dm_activity_until.clone_from(&self.unusual_dm_activity_until);
member.set_pending(self.pending());
member.set_deaf(self.deaf());
member.set_mute(self.mute());

item
} else {
None
};

if item.is_none() {
guild.members.insert(self.user.id, Member {
deaf: false,
let mut new_member = Member {
__generated_flags: MemberGeneratedFlags::empty(),
guild_id: self.guild_id,
joined_at: Some(self.joined_at),
mute: false,
nick: self.nick.clone(),
roles: self.roles.clone(),
user: self.user.clone(),
pending: self.pending,
premium_since: self.premium_since,
permissions: None,
avatar: self.avatar,
communication_disabled_until: self.communication_disabled_until,
flags: GuildMemberFlags::default(),
unusual_dm_activity_until: self.unusual_dm_activity_until,
});
};

new_member.set_pending(self.pending());
new_member.set_deaf(self.deaf());
new_member.set_mute(self.mute());

guild.members.insert(self.user.id, new_member);
}

item
Expand Down Expand Up @@ -314,7 +318,7 @@ impl CacheUpdate for GuildUpdateEvent {
guild.system_channel_id = self.guild.system_channel_id;
guild.verification_level = self.guild.verification_level;
guild.widget_channel_id = self.guild.widget_channel_id;
guild.widget_enabled = self.guild.widget_enabled;
guild.set_widget_enabled(self.guild.widget_enabled());
}

None
Expand Down Expand Up @@ -412,20 +416,18 @@ impl CacheUpdate for PresenceUpdateEvent {
// Create a partial member instance out of the presence update data.
if let Some(user) = self.presence.user.to_user() {
guild.members.entry(self.presence.user.id).or_insert_with(|| Member {
deaf: false,
guild_id,
joined_at: None,
mute: false,
nick: None,
user,
roles: FixedArray::default(),
pending: false,
premium_since: None,
permissions: None,
avatar: None,
communication_disabled_until: None,
flags: GuildMemberFlags::default(),
unusual_dm_activity_until: None,
__generated_flags: MemberGeneratedFlags::empty(),
});
}
}
Expand Down
114 changes: 44 additions & 70 deletions src/framework/standard/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,24 +101,56 @@ impl From<(bool, bool, bool)> for WithWhiteSpace {
/// [`Client`]: crate::Client
/// [`StandardFramework`]: super::StandardFramework
/// [default implementation]: Self::default
#[bool_to_bitflags::bool_to_bitflags(
getter_prefix = "get_",
setter_prefix = "",
private_getters,
document_setters,
owning_setters
)]
#[derive(Clone)]
pub struct Configuration {
pub(crate) allow_dm: bool,
pub(crate) with_whitespace: WithWhiteSpace,
pub(crate) by_space: bool,
pub(crate) blocked_guilds: HashSet<GuildId>,
pub(crate) blocked_users: HashSet<UserId>,
pub(crate) allowed_channels: HashSet<ChannelId>,
pub(crate) disabled_commands: HashSet<String>,
pub(crate) dynamic_prefixes: Vec<DynamicPrefixHook>,
pub(crate) ignore_bots: bool,
pub(crate) ignore_webhooks: bool,
pub(crate) on_mention: Option<String>,
pub(crate) owners: HashSet<UserId>,
pub(crate) prefixes: Vec<String>,
pub(crate) no_dm_prefix: bool,
pub(crate) delimiters: Vec<Delimiter>,
pub(crate) case_insensitive: bool,
/// If set to false, bot will ignore any private messages.
///
/// **Note**: Defaults to `true`.
pub allow_dm: bool,
/// Whether the framework should split the message by a space first to parse the group or
/// command. If set to false, it will only test part of the message by the *length* of the
/// group's or command's names.
///
/// **Note**: Defaults to `true`
pub by_space: bool,
/// Whether the bot should respond to other bots.
///
/// For example, if this is set to false, then the bot will respond to any other bots including
/// itself.
///
/// **Note**: Defaults to `true`.
pub ignore_bots: bool,
/// If set to true, bot will ignore all commands called by webhooks.
///
/// **Note**: Defaults to `true`.
pub ignore_webhooks: bool,
/// Sets whether command execution can be done without a prefix. Works only in private
/// channels.
///
/// **Note**: Defaults to `false`.
///
/// # Note
///
/// The `cache` feature is required. If disabled this does absolutely nothing.
pub no_dm_prefix: bool,
case_insensitive: bool,
}

impl Configuration {
Expand All @@ -128,15 +160,6 @@ impl Configuration {
Self::default()
}

/// If set to false, bot will ignore any private messages.
///
/// **Note**: Defaults to `true`.
#[must_use]
pub fn allow_dm(mut self, allow_dm: bool) -> Self {
self.allow_dm = allow_dm;
self
}

/// Whether to allow whitespace being optional between a prefix/group-prefix/command and a
/// command.
///
Expand Down Expand Up @@ -165,17 +188,6 @@ impl Configuration {
self
}

/// Whether the framework should split the message by a space first to parse the group or
/// command. If set to false, it will only test part of the message by the *length* of the
/// group's or command's names.
///
/// **Note**: Defaults to `true`
#[must_use]
pub fn by_space(mut self, b: bool) -> Self {
self.by_space = b;
self
}

/// HashSet of channels Ids where commands will be working.
///
/// **Note**: Defaults to an empty HashSet.
Expand Down Expand Up @@ -351,27 +363,6 @@ impl Configuration {
self
}

/// Whether the bot should respond to other bots.
///
/// For example, if this is set to false, then the bot will respond to any other bots including
/// itself.
///
/// **Note**: Defaults to `true`.
#[must_use]
pub fn ignore_bots(mut self, ignore_bots: bool) -> Self {
self.ignore_bots = ignore_bots;
self
}

/// If set to true, bot will ignore all commands called by webhooks.
///
/// **Note**: Defaults to `true`.
#[must_use]
pub fn ignore_webhooks(mut self, ignore_webhooks: bool) -> Self {
self.ignore_webhooks = ignore_webhooks;
self
}

/// Whether or not to respond to commands initiated with `id_to_mention`.
///
/// **Note**: that this can be used in conjunction with [`Self::prefix`].
Expand Down Expand Up @@ -485,20 +476,6 @@ impl Configuration {
self
}

/// Sets whether command execution can be done without a prefix. Works only in private channels.
///
/// **Note**: Defaults to `false`.
///
/// # Note
///
/// The `cache` feature is required. If disabled this does absolutely nothing.
#[inline]
#[must_use]
pub fn no_dm_prefix(mut self, b: bool) -> Self {
self.no_dm_prefix = b;
self
}

/// Sets a single delimiter to be used when splitting the content after a command.
///
/// **Note**: Defaults to a vector with a single element of `' '`.
Expand Down Expand Up @@ -555,7 +532,7 @@ impl Configuration {
/// **Note**: Defaults to `false`.
#[must_use]
pub fn case_insensitivity(mut self, cs: bool) -> Self {
self.case_insensitive = cs;
self = self.case_insensitive(cs);

for prefix in &mut self.prefixes {
*prefix = prefix.to_lowercase();
Expand Down Expand Up @@ -585,23 +562,20 @@ impl Default for Configuration {
/// - **owners** to an empty HashSet
/// - **prefix** to "~"
fn default() -> Configuration {
Configuration {
allow_dm: true,
let config = Configuration {
__generated_flags: ConfigurationGeneratedFlags::empty(),
with_whitespace: WithWhiteSpace::default(),
by_space: true,
blocked_guilds: HashSet::default(),
blocked_users: HashSet::default(),
allowed_channels: HashSet::default(),
case_insensitive: false,
delimiters: vec![Delimiter::Single(' ')],
disabled_commands: HashSet::default(),
dynamic_prefixes: Vec::new(),
ignore_bots: true,
ignore_webhooks: true,
no_dm_prefix: false,
on_mention: None,
owners: HashSet::default(),
prefixes: vec![String::from("~")],
}
};

config.allow_dm(true).by_space(true).ignore_bots(true).ignore_webhooks(true)
}
}
8 changes: 4 additions & 4 deletions src/framework/standard/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ impl StandardFramework {
fn should_ignore(&self, msg: &Message) -> bool {
let config = self.config.read();

(config.ignore_bots && msg.author.bot)
|| (config.ignore_webhooks && msg.webhook_id.is_some())
(config.get_ignore_bots() && msg.author.bot())
|| (config.get_ignore_webhooks() && msg.webhook_id.is_some())
}

async fn should_fail<'a>(
Expand Down Expand Up @@ -636,7 +636,7 @@ impl Framework for StandardFramework {
return;
}

if prefix.is_none() && !(config.no_dm_prefix && msg.is_private()) {
if prefix.is_none() && !(config.get_no_dm_prefix() && msg.is_private()) {
if let Some(normal) = &self.normal_message {
normal(&mut ctx, &msg).await;
}
Expand Down Expand Up @@ -683,7 +683,7 @@ impl Framework for StandardFramework {

match invoke {
Invoke::Help(name) => {
if !config.allow_dm && msg.is_private() {
if !config.get_allow_dm() && msg.is_private() {
return;
}

Expand Down
7 changes: 5 additions & 2 deletions src/framework/standard/parse/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ impl CommandMap {
map.min_length = std::cmp::min(len, map.min_length);
map.max_length = std::cmp::max(len, map.max_length);

let name =
if conf.case_insensitive { name.to_lowercase() } else { (*name).to_string() };
let name = if conf.get_case_insensitive() {
name.to_lowercase()
} else {
(*name).to_string()
};

map.cmds.insert(name, (*cmd, Arc::clone(&sub_map)));
}
Expand Down
Loading

0 comments on commit 8fca50e

Please sign in to comment.