Entity mapping race condition when receiving entity-ref component before associated entity is spawned

[philpax]

If you have two entities, A and B, where B contains a component with add_map_entities pointing to A, and B is received before A, the ID that B contains of A will be the server's ID and not the mapped ID.

The following systems on the server (simplified):

pub fn plugin(app: &mut App) {
    app.init_resource::<DelayedSpawns>().add_systems(
        Update,
        (
            handle_connections,
            delayed_spawns.run_if(on_timer(Duration::from_secs(30))),
        )
            .chain(),
    );
}

#[derive(Resource, Default)]
struct DelayedSpawns(Vec<(Entity, String)>);

fn handle_connections(
    client_id_map: Res<ClientIdMap>,
    mut delayed_spawns: ResMut<DelayedSpawns>,
    mut connections: EventReader<ConnectEvent>,
    mut commands: Commands,
) {
    for connection in connections.read() {
        let client_id = connection.client_id;

        let Some(user_id) = client_id_map.remove_pending_client(client_id) else {
            warn!("client({client_id:?}): no pending session");
            // TODO: handle disconnect
            continue;
        };

        let entity = commands
            .spawn((
                Name::new(format!("Player {user_id}")),
                NetPlayer,
                PlayerId(client_id),
                PlayerUserId(user_id.clone()),
                Replicate {
                    target: ReplicationTarget {
                        target: NetworkTarget::All,
                    },
                    sync: SyncTarget {
                        prediction: NetworkTarget::None,
                        interpolation: NetworkTarget::None,
                    },
                    controlled_by: ControlledBy {
                        target: NetworkTarget::Single(client_id),
                        ..default()
                    },
                    group: ReplicationGroup::new_id(client_id.to_bits()),
                    ..default()
                },
            ))
            .id();
        client_id_map.add_connected_client(client_id, entity, user_id.clone());
        info!("client({client_id:?}): {user_id} spawn {}", entity);

        commands.spawn((
            Name::new(format!("Test {user_id} - Same Tick")),
            OwnedByPlayer(entity),
            Replicate::default(),
        ));

        delayed_spawns.0.push((entity, user_id.clone()));
    }
}

fn delayed_spawns(mut delayed_spawns: ResMut<DelayedSpawns>, mut commands: Commands) {
    for (entity, user_id) in delayed_spawns.0.drain(..) {
        commands.spawn((
            Name::new(format!("Test {user_id} - Later Tick")),
            OwnedByPlayer(entity),
            Replicate::default(),
        ));
    }
}

and on the client:

fn receive_entity_spawn(
    name_query: Query<&Name>,
    player_user_id_query: Query<&PlayerUserId>,
    mut reader: EventReader<EntitySpawnEvent>,
    mut toasts: ResMut<Toasts>,
) {
    for event in reader.read() {
        let entity = event.entity();
        if let Ok(name) = name_query.get(entity).map(|e| e.as_str()) {
            info!("Received named entity spawn {entity}: {name}");
        }
    }
}

fn print_owned_by_player_with_names(
    query: Query<(&OwnedByPlayer, &Name), Added<OwnedByPlayer>>,
    player_query: Query<&PlayerUserId>,
) {
    for (owned_by_player, name) in query.iter() {
        info!(
            "{:?} (Name {:?}) (exists: {})",
            owned_by_player,
            name,
            player_query.contains(owned_by_player.0)
        );
    }
}

with this component registration (apologies, macro-generated code):

#[derive(
    Component, Reflect, serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq, Copy,
)]
#[reflect(Component)]
#[doc = r" This entity is owned by this player."]
pub struct OwnedByPlayer(pub Entity);
impl bevy::ecs::entity::MapEntities for OwnedByPlayer {
    fn map_entities<M: bevy::ecs::entity::EntityMapper>(&mut self, entity_mapper: &mut M) {
        self.0 = entity_mapper.map_entity(self.0);
    }
}
fn register_entity_ref_components_full_sync(app: &mut App) {
    use lightyear::{
        channel::builder::ChannelDirection, client::components::ComponentSyncMode,
        prelude::AppComponentExt,
    };
    app.register_type::<OwnedByPlayer>()
        .register_component::<OwnedByPlayer>(ChannelDirection::Bidirectional)
        .add_prediction(ComponentSyncMode::Once)
        .add_interpolation(ComponentSyncMode::Once)
        .add_map_entities();
}

produce the following logs (trimmed):

server:
2024-10-02T23:11:20.158659Z INFO server_lib::player | client(Netcode(12389886290396887533)): philpax1 spawn 580v1
2024-10-02T23:12:43.453054Z INFO server_lib::player | client(Netcode(16023685332025805423)): philpax2 spawn 531v2

player1:
2024-10-02T23:11:20.171505Z INFO client | Connected to server with client ID Netcode(12389886290396887533)
2024-10-02T23:11:20.915702Z INFO client | OwnedByPlayer(Entity { index: 580, generation: 1 }) (Name "Test philpax1 - Same Tick") (exists: false)
2024-10-02T23:11:20.915724Z INFO client | Received named entity spawn 413v1: Test philpax1 - Same Tick
2024-10-02T23:11:20.915781Z INFO client | Received named entity spawn 415v1: Player philpax1
2024-10-02T23:12:01.013873Z INFO client | Received named entity spawn 519v2: Test philpax1 - Later Tick
2024-10-02T23:12:01.013866Z INFO client | OwnedByPlayer(Entity { index: 415, generation: 1 }) (Name "Test philpax1 - Later Tick") (exists: true)
2024-10-02T23:12:43.964412Z INFO client | OwnedByPlayer(Entity { index: 531, generation: 2 }) (Name "Test philpax2 - Same Tick") (exists: false)
2024-10-02T23:12:43.964426Z INFO client | Received named entity spawn 378v2: Test philpax2 - Same Tick
2024-10-02T23:12:45.551558Z INFO client | Received named entity spawn 486v2: Player philpax2
2024-10-02T23:12:55.604639Z INFO client | OwnedByPlayer(Entity { index: 486, generation: 2 }) (Name "Test philpax2 - Later Tick") (exists: true)
2024-10-02T23:12:55.604641Z INFO client | Received named entity spawn 524v2: Test philpax2 - Later Tick

player2:
2024-10-02T23:12:43.456302Z INFO client | Connected to server with client ID Netcode(16023685332025805423)
2024-10-02T23:12:44.058177Z INFO client | OwnedByPlayer(Entity { index: 580, generation: 1 }) (Name "Test philpax1 - Later Tick") (exists: false)
2024-10-02T23:12:44.058249Z INFO client | OwnedByPlayer(Entity { index: 580, generation: 1 }) (Name "Test philpax1 - Same Tick") (exists: false)
2024-10-02T23:12:44.058264Z INFO client | OwnedByPlayer(Entity { index: 531, generation: 2 }) (Name "Test philpax2 - Same Tick") (exists: false)
2024-10-02T23:12:44.058195Z INFO client | Received named entity spawn 65v1: Test philpax1 - Later Tick
2024-10-02T23:12:44.058308Z INFO client | Received named entity spawn 96v1: Test philpax1 - Same Tick
2024-10-02T23:12:44.058314Z INFO client | Received named entity spawn 131v1: Test philpax2 - Same Tick
2024-10-02T23:12:44.402739Z INFO client | Received named entity spawn 173v1: Player philpax1
2024-10-02T23:12:44.411357Z INFO client | Received named entity spawn 174v1: Player philpax2
2024-10-02T23:12:54.791470Z INFO client | OwnedByPlayer(Entity { index: 174, generation: 1 }) (Name "Test philpax2 - Later Tick") (exists: true)
2024-10-02T23:12:54.791490Z INFO client | Received named entity spawn 235v1: Test philpax2 - Later Tick

That is, the Test entities with the OwnedByPlayer component that are received the same, or previous tick, to the actual player entity receive the server's entity ID. Delaying the spawn so that the player entities have definitely been replicated to the client results in the ref being resolved correctly.

I'm not sure what the correct fix would be here - delaying the addition of the component until a mapping can be established, perhaps?

(Same behaviour with #656 as well)

[cBournhonesque]

I haven't followed to closely all the logs, but it sounds like you want to guarantee that A is received before B? Is it ok if A and B are replicated together?

You can use ReplicationGroups to guarantee that multiple entities will be part of the same packet. You just need to give the same ReplicationGroupId to both entities

[philpax]

Oh, I didn't realise that would work - my use case was attaching OwnedByPlayer to client-replicated entities so that clients could join other-clients'-owned-entities to their player entity, and I assumed that ReplicationGroup wouldn't work for that due to timing. Seems to work on LAN, so I'll close this unless I encounter this on a real network - thanks again for the guidance :pray: