Jondolf / avian

ECS-driven 2D and 3D physics engine for the Bevy game engine.
https://crates.io/crates/avian3d
Apache License 2.0
1.55k stars 120 forks source link

Use a single layer as the default membership instead of all. #476

Closed Aceeri closed 3 months ago

Aceeri commented 3 months ago

Objective

Using ALL memberships requires a lot more effort to correctly use the layer system in gameplay, since interactions you wanted exclusive to one object now trigger on all defaulted objects as well.

This system is similar to those used in Box2d (one memberships, all filters) and Godot (one memberships, one filters), ~however the current system is the same as Rapier and Unity (all memberships, all filters).~ Unity is actually exclusively one membership, all filters with configurable filters in your project.

Solution

Additional Questions

~- Should we use the default layer for filters as well?~ No, this doesn't make sense unless we swap to using || for the calculation and/or allow default configuration as it forbids some valid usecases like a default member not interacting with default.

Changelog

Migration Guide

Aceeri commented 3 months ago

Bit of a footgun regarding implicitness, but then again Bevy does the exact same thing with RenderLayers, so we are at least consistent. In any case, this is way better then the current behavior, as I can just tell my Sensors to filter out CollisionLayers::DEFAULT.

It's a footgun the current way too since you cannot actually ignore the default objects ever. E.g. you shape cast for a dumpster or interactable infront of the player, but it'll just return any physics object it touches aside from custom ones.

janhohenheim commented 3 months ago

@Aceeri yes, I definitely agree

Aceeri commented 3 months ago

I've let this sit for a while (sorry about that!) but I'm largely in favor of this. I pushed a quick commit to fix some docs, and to add a Default variant to the enums, since imo it doesn't make sense for the Ground layer to be the default layer that everything belongs to. I hope that's fine; if you disagree on something, we can revert.

One thing I don't really like is how implicit the default is with the enum-based approach. For a follow-up, I think it could be interesting to require users to explicitly derive Default and specify the default layer with the #[default] attribute. I have this implemented already, so I'll put up a PR for consideration after this is merged.

Yeah I was considering that might be better and initially just changed the derive macro start at 2 instead of 1, but I agree with all those changes.