Skip to content

Commit

Permalink
less Arc<S> in brigadier
Browse files Browse the repository at this point in the history
  • Loading branch information
@mat-1
mat-1 committed Oct 13, 2023
1 parent 38db231 commit d82230a
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 27 deletions.
6 changes: 3 additions & 3 deletions azalea-brigadier/src/builder/argument_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub struct ArgumentBuilder<S> {
arguments: CommandNode<S>,

command: Command<S>,
requirement: Arc<dyn Fn(Arc<S>) -> bool + Send + Sync>,
requirement: Arc<dyn Fn(&S) -> bool + Send + Sync>,
target: Option<Arc<RwLock<CommandNode<S>>>>,

forks: bool,
Expand Down Expand Up @@ -96,13 +96,13 @@ impl<S> ArgumentBuilder<S> {
/// # let mut subject = CommandDispatcher::<CommandSource>::new();
/// # subject.register(
/// literal("foo")
/// .requires(|s: Arc<CommandSource>| s.opped)
/// .requires(|s: &CommandSource| s.opped)
/// // ...
/// # .executes(|ctx: &CommandContext<CommandSource>| 42)
/// # );
pub fn requires<F>(mut self, requirement: F) -> Self
where
F: Fn(Arc<S>) -> bool + Send + Sync + 'static,
F: Fn(&S) -> bool + Send + Sync + 'static,
{
self.requirement = Arc::new(requirement);
self
Expand Down
31 changes: 13 additions & 18 deletions azalea-brigadier/src/command_dispatcher.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl<S> CommandDispatcher<S> {
let cursor = original_reader.cursor();

for child in node.read().get_relevant_nodes(&mut original_reader.clone()) {
if !child.read().can_use(source.clone()) {
if !child.read().can_use(&source) {
continue;
}
let mut context = context_so_far.clone();
Expand Down Expand Up @@ -307,7 +307,7 @@ impl<S> CommandDispatcher<S> {
pub fn get_all_usage(
&self,
node: &CommandNode<S>,
source: Arc<S>,
source: &S,
restricted: bool,
) -> Vec<String> {
let mut result = vec![];
Expand All @@ -318,12 +318,12 @@ impl<S> CommandDispatcher<S> {
fn get_all_usage_recursive(
&self,
node: &CommandNode<S>,
source: Arc<S>,
source: &S,
result: &mut Vec<String>,
prefix: &str,
restricted: bool,
) {
if restricted && !node.can_use(source.clone()) {
if restricted && !node.can_use(&source) {

Check warning on line 326 in azalea-brigadier/src/command_dispatcher.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

warning: this expression creates a reference which is immediately dereferenced by the compiler
   --> azalea-brigadier/src/command_dispatcher.rs:326:40
    |
326 |         if restricted && !node.can_use(&source) {
    |                                        ^^^^^^^ help: change this to: `source`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
    = note: `#[warn(clippy::needless_borrow)]` on by default
return;
}
if node.command.is_some() {
Expand All @@ -345,7 +345,7 @@ impl<S> CommandDispatcher<S> {
let child = child.read();
self.get_all_usage_recursive(
&child,
Arc::clone(&source),
&source,

Check warning on line 348 in azalea-brigadier/src/command_dispatcher.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

warning: this expression creates a reference which is immediately dereferenced by the compiler
   --> azalea-brigadier/src/command_dispatcher.rs:348:21
    |
348 |                     &source,
    |                     ^^^^^^^ help: change this to: `source`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
result,
if prefix.is_empty() {
child.usage_text()
Expand All @@ -366,14 +366,13 @@ impl<S> CommandDispatcher<S> {
pub fn get_smart_usage(
&self,
node: &CommandNode<S>,
source: Arc<S>,
source: &S,
) -> Vec<(Arc<RwLock<CommandNode<S>>>, String)> {
let mut result = Vec::new();

let optional = node.command.is_some();
for child in node.children.values() {
let usage =
self.get_smart_usage_recursive(&child.read(), source.clone(), optional, false);
let usage = self.get_smart_usage_recursive(&child.read(), source, optional, false);
if let Some(usage) = usage {
result.push((child.clone(), usage));
}
Expand All @@ -385,11 +384,11 @@ impl<S> CommandDispatcher<S> {
fn get_smart_usage_recursive(
&self,
node: &CommandNode<S>,
source: Arc<S>,
source: &S,
optional: bool,
deep: bool,
) -> Option<String> {
if !node.can_use(source.clone()) {
if !node.can_use(&source) {

Check warning on line 391 in azalea-brigadier/src/command_dispatcher.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

warning: this expression creates a reference which is immediately dereferenced by the compiler
   --> azalea-brigadier/src/command_dispatcher.rs:391:26
    |
391 |         if !node.can_use(&source) {
    |                          ^^^^^^^ help: change this to: `source`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
return None;
}

Expand Down Expand Up @@ -418,14 +417,14 @@ impl<S> CommandDispatcher<S> {
let children = node
.children
.values()
.filter(|child| child.read().can_use(source.clone()))
.filter(|child| child.read().can_use(&source))

Check warning on line 420 in azalea-brigadier/src/command_dispatcher.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

warning: this expression creates a reference which is immediately dereferenced by the compiler
   --> azalea-brigadier/src/command_dispatcher.rs:420:50
    |
420 |             .filter(|child| child.read().can_use(&source))
    |                                                  ^^^^^^^ help: change this to: `source`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
.collect::<Vec<_>>();
match children.len().cmp(&1) {
Ordering::Less => {}
Ordering::Equal => {
let usage = self.get_smart_usage_recursive(
&children[0].read(),
source.clone(),
source,
child_optional,
child_optional,
);
Expand All @@ -436,12 +435,8 @@ impl<S> CommandDispatcher<S> {
Ordering::Greater => {
let mut child_usage = HashSet::new();
for child in &children {
let usage = self.get_smart_usage_recursive(
&child.read(),
source.clone(),
child_optional,
true,
);
let usage =
self.get_smart_usage_recursive(&child.read(), source, child_optional, true);
if let Some(usage) = usage {
child_usage.insert(usage);
}
Expand Down
4 changes: 2 additions & 2 deletions azalea-brigadier/src/tree/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub struct CommandNode<S> {
pub arguments: HashMap<String, Arc<RwLock<CommandNode<S>>>>,

pub command: Command<S>,
pub requirement: Arc<dyn Fn(Arc<S>) -> bool + Send + Sync>,
pub requirement: Arc<dyn Fn(&S) -> bool + Send + Sync>,
pub redirect: Option<Arc<RwLock<CommandNode<S>>>>,
pub forks: bool,
pub modifier: Option<Arc<RedirectModifier<S>>>,
Expand Down Expand Up @@ -97,7 +97,7 @@ impl<S> CommandNode<S> {
}
}

pub fn can_use(&self, source: Arc<S>) -> bool {
pub fn can_use(&self, source: &S) -> bool {
(self.requirement)(source)
}

Expand Down
8 changes: 4 additions & 4 deletions azalea-brigadier/tests/command_dispatcher_usages_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,21 +81,21 @@ fn get(subject: &CommandDispatcher<()>, command: &str) -> Arc<RwLock<CommandNode
#[test]
fn test_all_usage_no_commands() {
let subject = CommandDispatcher::<()>::new();
let results = subject.get_all_usage(&subject.root.read(), Arc::new(()), true);
let results = subject.get_all_usage(&subject.root.read(), &(), true);
assert!(results.is_empty());
}

#[test]
fn test_smart_usage_no_commands() {
let subject = CommandDispatcher::<()>::new();
let results = subject.get_smart_usage(&subject.root.read(), Arc::new(()));
let results = subject.get_smart_usage(&subject.root.read(), &());
assert!(results.is_empty());
}

#[test]
fn test_all_usage_root() {
let subject = setup();
let results = subject.get_all_usage(&subject.root.read(), Arc::new(()), true);
let results = subject.get_all_usage(&subject.root.read(), &(), true);

let actual = results.into_iter().collect::<HashSet<_>>();
let expected = vec![
Expand All @@ -112,7 +112,7 @@ fn test_all_usage_root() {
#[test]
fn test_smart_usage_root() {
let subject = setup();
let results = subject.get_smart_usage(&subject.root.read(), Arc::new(()));
let results = subject.get_smart_usage(&subject.root.read(), &());

let actual = results
.into_iter()
Expand Down

0 comments on commit d82230a

Please sign in to comment.