-
Notifications
You must be signed in to change notification settings - Fork 203
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(spells): support empty trigger configs [NET-316] #1412
Conversation
An empty trigger config is used either to unsubscribe a spell from triggers or to not subscribe at all if it was used on installation. Also remove updating interface inside the spell-event-bus, since it's not needed.
#[test] | ||
fn spell_update_config() { | ||
let swarms = make_swarms(1); | ||
//log_utils::enable_logs(); |
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.
//log_utils::enable_logs(); |
)"#, | ||
client.peer_id | ||
); | ||
// create periodic spell |
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.
// create periodic spell |
#[test] | ||
fn spell_update_config_stopped_spell() { | ||
let swarms = make_swarms(1); | ||
//log_utils::enable_logs(); |
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.
//log_utils::enable_logs(); |
.wrap_err("connect client") | ||
.unwrap(); | ||
|
||
// Create spell that do nothing. |
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.
// Create spell that do nothing. | |
// Create a spell that does nothing. |
} | ||
was_removed |
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.
nice!
let new_len = self.scheduled.len(); | ||
let removed = self.subscribers.remove(spell_id); | ||
prev_len != new_len || removed | ||
} | ||
|
||
fn update(&mut self, spell_id: &SpellId, config: &SpellTriggerConfigs) { | ||
if self.unsubscribe(spell_id) { | ||
self.subscribe(spell_id.clone(), config); | ||
} |
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.
awesome ^__^
sorcerer/src/spells.rs
Outdated
))); | ||
} | ||
} else { | ||
log::trace!("empty config given for spell {}", spell_id); |
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.
can you include particle_id in this log please? I think one day we might want to see this message when debugging spell installation
Co-authored-by: Aleksey Proshutisnkiy <justprosh@users.noreply.github.com>
An empty trigger config allows:
Also, remove the update interface from the spell-event-bus crate since all it needs to do is expressed via subscribe/unsubscribe methods.