laundmo / bevy-spatial

Spatial datastructures for Bevy
Other
150 stars 17 forks source link

Suggestion: Changes to support augmented Queries as SystemParam #28

Closed IronGremlin closed 11 months ago

IronGremlin commented 11 months ago

"Problem":

Currently, the SpatialAccess trait expresses the return result of any operations as Self::ResultT - which means that any code anyone writes that wants to be generic over implementers of SpatialAccess doesn't get any kind of knowledge about what might be contained in the results of any of the operations -

Specifically, there's no way to write code against SpatialAccess as a trait that also "understands" how to access any entities returned in any of the operations exposed by SpatialAccess

Here's an example motivating use-case:

#[derive(SystemParam)]
pub struct DistanceAwareQuery<'w, 's, Comp, Q, F = ()>
where
    Q: WorldQuery + 'static,
    F: ReadOnlyWorldQuery + 'static,
    Comp: Component,
{
    all_t: Query<'w, 's, Q, (F, With<Comp>)>,
    space: Res<'w, KDTree2<Comp>>,
}

impl<'w, 's, Comp, Q, F> DistanceAwareQuery<'w, 's, Comp, Q, F>
where
    Q: WorldQuery + 'static,
    F: ReadOnlyWorldQuery + 'static,
    Comp: Component,
{
    pub fn within_distance(
        &self,
        loc: Vec2,
        distance: f32,
    ) -> QueryManyIter<
        '_,
        's,
        <Q as WorldQuery>::ReadOnly,
        (F, With<Comp>),
        <Vec<Entity> as IntoIterator>::IntoIter,
    > {
        let spacevec: Vec<Entity> = self
            .space
            .within_distance(loc, distance)
            .iter()
            .filter_map(|(_, x)| *x)
            .collect();

        self.all_t.iter_many(spacevec)
    }

    pub fn within_distance_mut(
        &mut self,
        loc: Vec2,
        distance: f32,
    ) -> QueryManyIter<'_, 's, Q, (F, With<Comp>), <Vec<Entity> as IntoIterator>::IntoIter> {
        let spacevec: Vec<Entity> = self
            .space
            .within_distance(loc, distance)
            .iter()
            .filter_map(|(_, x)| *x)
            .collect();

        self.all_t.iter_many_mut(spacevec)
    }
}

#[derive(Component)]
pub struct Burning;

fn catch_fire(
    mut commands: Commands,

    on_fire: Query<&GlobalTransform, With<Burning>>,
    spreading: DistanceAwareQuery<SpatialMarker, Entity, Without<Burning>>,
) {
    on_fire.iter().for_each(|transform| {
        spreading
            .within_distance(transform.translation().xy(), 1.0)
            .for_each(|entity| {
                commands.entity(entity).insert(Burning);
            })
    })
}

See also this lonely rant on discord

The "problem" with the currently implementation is that, because we cannot extract any entities from any calls to within_distance, we can't augment the results of the query that we're constructing without specifying that we're accessing a KDTree2 - if we could recover how to do that from the return signature in the trait definition, then we could instead define all of this in terms of some user supplied type implementing SpatialAccess + Resource.

I'm not sure this would ever actually be a problem to anyone who was not attempting to do more or less exactly what I am attempting to do here - and although my own usecase feels especially compelling to me, I lack a sense of how compelling it might be to others.

Is there any interest in a PR here, either to "fix" this one narrow issue or to expose a similar construct from bevy_spatial?

laundmo commented 11 months ago

Hi, thanks for this. I understand your usecase, and normally a PR would be welcome, but i'm afraid it might be wasted right now. I'm planning a rewrite/big update, to use Components instead of Resources and rely more on individual systems interacting than the rather complex Traits/Generics used right now. This would come along with 2 new datastructures, a simple hashmap and spatial grid, which i've comitted (very unfinished) just earlier today to the branch https://github.com/laundmo/bevy-spatial/tree/grid-and-hash

Thank you for your concern, and i'll see about making this possible or even built in within that rewrite.

IronGremlin commented 11 months ago

Thanks for taking the time to respond -

Exciting stuff coming up! I'm happy to see grid support as well, my project currently implements an incredibly sad and slapdash workaround for that exact challenge and I'm very enthused to have a solid excuse to kill that code with fire.

Happy hacking!

laundmo commented 11 months ago

Exciting stuff coming up! I'm happy to see grid support as well

I'm afraid theres been a misundertsanding: a spatial grid is a datastructure used for the same purpose, to allow queries on distributed points. its not meant for uniform grids, just happens to be one.