Skip to content

Commit

Permalink
Fix: incompatible component access issue for gt 2 queries
Browse files Browse the repository at this point in the history
This commit resolves an issue where queries with different component access
expressions were incorrectly identified as incompatible when used together in
an event handler. The problem was rooted in the logic that combined the component
access expressions: it failed to account for the scenario where multiple queries
in a handler could be compatible individually but not when combined.
  • Loading branch information
andrewgazelka committed Mar 25, 2024
1 parent 6f51164 commit a5d6d58
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 24 deletions.
1 change: 0 additions & 1 deletion src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,6 @@ impl Archetype {
fn register_handler(&mut self, info: &mut HandlerInfo) {
if info
.component_access()
.expr
.eval(|idx| self.column_of(idx).is_some())
{
if self.entity_count() > 0 {
Expand Down
8 changes: 2 additions & 6 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -904,9 +904,7 @@ unsafe impl<E: Event, Q: Query + 'static> HandlerParam for Receiver<'_, E, Q> {

config.targeted_event_expr = expr.expr.clone();

if let Ok(new_component_access) = expr.or(&config.component_access) {
config.component_access = new_component_access;
} else {
if config.try_add_component_access(expr).is_err() {
return Err(InitError(
format!(
"query `{}` has incompatible component access with previous queries in this \
Expand Down Expand Up @@ -1022,9 +1020,7 @@ unsafe impl<E: Event, Q: Query + 'static> HandlerParam for ReceiverMut<'_, E, Q>

config.targeted_event_expr = expr.expr.clone();

if let Ok(new_component_access) = expr.or(&config.component_access) {
config.component_access = new_component_access;
} else {
if config.try_add_component_access(expr).is_err() {
return Err(InitError(
format!(
"query `{}` has incompatible component access with previous queries in this \
Expand Down
21 changes: 9 additions & 12 deletions src/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,15 @@ impl<Q: Query> FetcherState<Q> {

let res = FetcherState::new(state);

match expr.or(&config.component_access) {
Ok(new_component_access) => config.component_access = new_component_access,
Err(_) => {
return Err(InitError(
format!(
"query `{}` has incompatible component access with previous queries in \
this handler",
any::type_name::<Q>()
)
.into(),
))
}
if config.try_add_component_access(expr).is_err() {
return Err(InitError(
format!(
"query `{}` has incompatible component access with previous queries in this \
handler",
any::type_name::<Q>()
)
.into(),
));
}

Ok(res)
Expand Down
26 changes: 22 additions & 4 deletions src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ pub(crate) struct HandlerInfoInner<H: ?Sized = dyn Handler> {
pub(crate) sent_untargeted_events: BitSet<UntargetedEventIdx>,
pub(crate) sent_targeted_events: BitSet<TargetedEventIdx>,
pub(crate) event_queue_access: Access,
pub(crate) component_access: ComponentAccessExpr,
pub(crate) component_access: BoolExpr<ComponentIdx>,
pub(crate) referenced_components: BitSet<ComponentIdx>,
pub(crate) priority: Priority,
// SAFETY: There is intentionally no public accessor for this field as it would lead to mutable
Expand Down Expand Up @@ -392,7 +392,7 @@ impl HandlerInfo {
}

/// Gets the expression describing this handler's access
pub fn component_access(&self) -> &ComponentAccessExpr {
pub fn component_access(&self) -> &BoolExpr<ComponentIdx> {
unsafe { &(*AliasedBox::as_ptr(&self.0)).component_access }
}

Expand Down Expand Up @@ -858,7 +858,11 @@ pub struct Config {
/// Access to the queue of events.
pub event_queue_access: Access,
/// Expression describing the components accessed by the handler.
pub component_access: ComponentAccessExpr,
pub component_access_expr: BoolExpr<ComponentIdx>,
/// Multiple component access expressions, one for each accessor (Fetcher,
/// Receiver, etc).
pub multiple_component_access: Vec<ComponentAccessExpr>,

/// The set of components referenced by this handler. Used for handler
/// cleanup when a component is removed.
///
Expand All @@ -880,10 +884,24 @@ impl Config {
sent_untargeted_events: Default::default(),
sent_targeted_events: Default::default(),
event_queue_access: Default::default(),
component_access: ComponentAccessExpr::new(false),
component_access_expr: BoolExpr::new(false),
multiple_component_access: Vec::default(),
referenced_components: Default::default(),
}
}

pub(crate) fn try_add_component_access(
&mut self,
expr: ComponentAccessExpr,
) -> Result<(), ComponentAccessExpr> {
for access in &self.multiple_component_access {
expr.clone().or(access)?;
}

self.multiple_component_access.push(expr.clone());
self.component_access_expr = expr.expr.or(&self.component_access_expr);
Ok(())
}
}

impl Default for Config {
Expand Down
2 changes: 1 addition & 1 deletion src/world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ impl World {
sent_untargeted_events: config.sent_untargeted_events,
sent_targeted_events: config.sent_targeted_events,
event_queue_access: config.event_queue_access,
component_access: config.component_access,
component_access: config.component_access_expr,
referenced_components: config.referenced_components,
priority: config.priority,
handler,
Expand Down

0 comments on commit a5d6d58

Please sign in to comment.