fix so cursor packet is sent to client with open inventories

[lukashermansson]

Objective

Attempts to solve the issue shown in discord https://discord.com/channels/998132822239870997/1292890441011957830
where doing has no effect in a "open inventory"

Looks like an oversight in the system to update the open inventories, no attempt to send any packet to the user was made when the CursorItem component was updated by any system

Playground to reproduce the issue solved by this pr.

use valence::interact_block::InteractBlockEvent;
use valence::inventory::ClickSlotEvent;
use valence::log::LogPlugin;
use valence::prelude::*;

#[allow(unused_imports)]
use crate::extras::*;

const SPAWN_Y: i32 = 64;
const CHEST_POS: [i32; 3] = [0, SPAWN_Y + 1, 3];

pub(crate) fn build_app(app: &mut App) {
    app.insert_resource(NetworkSettings {
        connection_mode: ConnectionMode::Offline,
        ..Default::default()
    })
    .add_plugins(DefaultPlugins.build().disable::<LogPlugin>())
    .add_systems(Startup, setup)
    .add_systems(
        EventLoopUpdate,
        (toggle_gamemode_on_sneak, open_chest, select_menu_item),
    )
    .add_systems(Update, (init_clients, despawn_disconnected_clients))
    .run();
}

fn setup(
    mut commands: Commands,
    server: Res<Server>,
    biomes: Res<BiomeRegistry>,
    dimensions: Res<DimensionTypeRegistry>,
) {
    let mut layer = LayerBundle::new(ident!("overworld"), &dimensions, &biomes, &server);

    for z in -5..5 {
        for x in -5..5 {
            layer.chunk.insert_chunk([x, z], UnloadedChunk::new());
        }
    }

    for z in -25..25 {
        for x in -25..25 {
            layer
                .chunk
                .set_block([x, SPAWN_Y, z], BlockState::GRASS_BLOCK);
        }
    }
    layer.chunk.set_block(CHEST_POS, BlockState::CHEST);

    commands.spawn(layer);

    let mut inventory = Inventory::new(InventoryKind::Generic9x3);
    inventory.set_slot(1, ItemStack::new(ItemKind::Apple, 64, None));
    inventory.set_slot(2, ItemStack::new(ItemKind::Diamond, 40, None));
    inventory.set_slot(3, ItemStack::new(ItemKind::Diamond, 30, None));
    commands.spawn(inventory);
}

fn init_clients(
    mut clients: Query<
        (
            &mut EntityLayerId,
            &mut VisibleChunkLayer,
            &mut VisibleEntityLayers,
            &mut Position,
        ),
        Added<Client>,
    >,
    layers: Query<Entity, (With<ChunkLayer>, With<EntityLayer>)>,
) {
    for (mut layer_id, mut visible_chunk_layer, mut visible_entity_layers, mut pos) in &mut clients
    {
        let layer = layers.single();

        layer_id.0 = layer;
        visible_chunk_layer.0 = layer;
        visible_entity_layers.0.insert(layer);
        pos.set([0.0, f64::from(SPAWN_Y) + 1.0, 0.0]);
    }
}

fn open_chest(
    mut commands: Commands,
    inventories: Query<Entity, (With<Inventory>, Without<Client>)>,
    mut events: EventReader<InteractBlockEvent>,
) {
    for event in events.read() {
        if event.position != CHEST_POS.into() {
            continue;
        }
        let open_inventory = OpenInventory::new(inventories.single());
        commands.entity(event.client).insert(open_inventory);
    }
}
fn select_menu_item(
    mut clients: Query<(Entity, &mut CursorItem)>,
    mut events: EventReader<ClickSlotEvent>,
) {
    for event in events.read() {
        let Ok((_player, mut cursor_item)) = clients.get_mut(event.client) else {
            continue;
        };

        // this does not work properly
        cursor_item.set_if_neq(CursorItem(ItemStack::EMPTY));
    }
}
// Add more systems here!

Solution

Copied the approach to sync the CursorItem state from the update_player_inventories system to the update_open_inventories where it was previously left out (my assumption is that it was left out by mistake).

[dyc3]

The tests will need to be updated.

[lukashermansson]

This has a test failure(as you also already spotted), don't merge this just yet, will need to get an implementation that does not send a packet to the client when it was the client that updated it! thanks for the quick review!

[dyc3]

nit: punctuation and clarity

Suggested change

	/// The item the user things they updated their cursor item to on the last
	/// tick if Some if none the user did not update their cursor item this
	/// last tick this is so we can inform the user of the update
	/// If `Some`: The item the user thinks they updated their cursor item to on the last
	/// tick.
	/// If `None`: the user did not update their cursor item in the last tick.
	/// This is so we can inform the user of the update through change detection.