Improved ergonomics for SceneEntityReference and #Name in the scene system

[camatvet]

Problem

SceneEntityReferences provide the very powerful utility of cross-scene entity references in scene construction, but are presently unwieldy without the bsn! macro, and when using the bsn! macro and the #Name notation, they cannot reference across scenes

Presently, SceneEntityReference wraps around a Hashed<InnerSceneEntityReference>, which has the following definition:

#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
pub struct InnerSceneEntityReference {
    file: &'static str,
    line: usize,
    column: usize,
    name_id: usize,
    runtime: u64,
}

The fields file, line, column, and runtime are not particularly meaningful outside of bsn! invocations; file, line and column are tied to a singular invocation site, one not present absent macros, and the _call_id that runtime is set to goes entirely unused without bsn!. To use it manually would entail constructions such as SceneEntityReference::new(("", 0, 0), n, 0), which is rather unwieldy. Within the goal of bsn! as an optional macro, it would be appropriate to make the great utility of SceneEntityReferences accessible without macros.

These fields then contribute to the inability of cross-macro references. Contrary to what one would reasonably expect from the following code, entity2 does not end up as the child of entity1:

// Line 20
let entity1 = commands.spawn_scene(bsn! {
    #Name
}).id();

// ...

// Line 40
let entity2 = commands.spawn_scene(bsn! {
    ChildOf(#Name)
}).id();

Because the InnerSceneEntityReference fields are contingent on the macro invocation, the #Names in entity1 and entity2 expand to different SceneEntityReferences and cannot be used to cross-reference.

Potential Solutions and Challenges

Looking through the code, it appears that the fields of InnerSceneEntityReference are only meaningfully used in the Display impl and the Hash impl. As such, it would be certainly feasible to refactor fields exclusive to debug info given that they get in the way of ergonomics.

I imagine InnerSceneEntityReference can be encoded entirely as an &'static str (or a String, I'm not sure) for a name, with an additional u64 for an index to distinguish with the same name, with the following definition:

#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
pub struct InnerSceneEntityReference {
    name: &'static str,
    index: u64
}

The name can operate similarly to the id attribute of an HTML item, although when unqualified, it comes with the risk of collisions.

let scenes = (0..10).map(|x| bsn! {
    #Slot
    InventorySlot({x})
}).collect::<Vec<_>>();

Within the current system, the above code would assign unique SceneEntityReferences to each element, owing to the increment of the call id, but introducing the assumption of a unique name within the scene would break scene composition within the construction of children.

One possible approach is to have the #Scene notation be annotated differently for macro-scoped and scene-scoped references, perhaps something like #Scene: for scene-scoped so as to leave the present macro-scoped functionality intact, and then #Scene:N to indicate N as the fixed index. For macro-scoped names, the global CALL_ID is the index. Scene-scoped names have the index as 0, unless N is specified, in which that becomes the index.

Outside the bsn! macro, SceneEntityReference would be constructed simply with a constructor, SceneEntityReference::new(name: &'static str, index: u64).

I hope a change like this would make the ergonomics of entity references within scenes more effective. This is the first time I've attempted to contribute to Bevy (or any open source project for that matter), so feedback on this proposal would be greatly appreciated.

[laundmo]

I've replied to this on discord already, where it was a numbered list of concerns/questions, expand below for that reply

Discord reply

As the person who implemented them:

1. Yeah, these fields are for the macro, see below for explanation
2. Yup, aware of this but theres no good plan right now to improve it, mostly because you're the first person i'm aware of asking for it to be usable manually. FYI: the main type for using entities in scenes is EntityTemplate, which has a variant you can use with just a Entity, which might help for manual usage.
3. This is fully intended and not something i'm aware of any plans existing to lift this. The way to act across scenes is by patching
4. The hash is the main reason they exist. The fields specifically exist to disambiguate #Name in macro invocation A from #Name in macro invocation B. Without this, just by accidentally using the same name, you'd make your entity a child of some other entity in the scene that may be entirely unrelated. Thats ultimately why the extra fields exist and the Hash impl is the important part here.
5. Not debug information, see 4.
6. The main purpose for SceneEntityReference is to allow for #Name syntax in bsn! to work, not much beyond that. Its not intended as the authoritative name<->entity mapping or anything like that, it only exists during spawn time
7. This is a mostly different feature that doesn't mesh well with the intended usage of SceneEntityReferences

I want to point out that the main reason for the way SceneEntityRefernce is implemented is to allow it to be passed to other scenes, specifically like bsn!{ #Name Children [ other_scene(#Name) ] } without causing issues.

I do believe theres value in some of the things you propose, but i'd urge you to consider alternative implementations instead of trying to shoehorn SceneEntityReferences to work for them. Therese a lot of ongoing work to improve bsn! and scenes, so i think if you wanted to share your actual usecases and wishes that'd be great.

In general: SceneEntityReferences are primarily an implementation detail of bsn! #Name entity references. The file/line/column/runtime fields exist to disambiguate entity references through the Hash impl relying on them, so that using the same named references across different scene functions (some of which may be coming from libraries) doesn't cause issues.

I think the better way forward is to decide on a design on how to solve the actual issues with bsn!/Scenes that this is attempting to solve. Its also likely ongoing work has already resolved some of this.