valence icon indicating copy to clipboard operation
valence copied to clipboard

Inventory Menus

Open dyc3 opened this issue 1 year ago • 4 comments

Describe the problem related to your feature request.

It's pretty common to (ab)use inventory windows to provide menu-like user interfaces.

What solution would you like?

We need to cover the following use cases:

  • [ ] A simple 1 page menu that has some buttons, eg. a menu that allows players to select minigame to play
  • [ ] A menu with an initial page, and you can navigate to sub pages, eg. pick a minigame, then pick a team to join
    • [ ] Unique window types for each page
  • [ ] A paginated list of items, eg. viewing a long list of search results
  • [ ] A paginated list of items that you can interact with like a regular inventory, eg. a really really big chest that has pagination to display all the items

We should have a new component InventoryMenu, that represents a menu.

This could go on the client's entity that has the menu open, similar to how OpenInventory works. This would allow users to render pages with customized information easier.

Alternatively, InventoryMenu could go on its own entity, similar to how Inventory works. This would allow menus to be cached and reused easier. This approach could end up looking something like this:

classDiagram
    direction TB
    class InventoryMenu {
        <<Component>>
        page: Vec~Page~

        + new()
        + with_pages(Vec~Page~ pages)
        - render(state: &InventoryMenuState)
    }

    class Page {
        buttons: HashMap "HashMap of slot to Button"
        kind: InventoryKind

        +new(kind: InventoryKind)
        +with_buttons(buttons)
        -render(): Vec~Option~ItemStack~~ 
    }

    class Button {
        item: ItemKind
        name: Option~Text~
        lore: Option~Vec~Text~~

        new()
        -render(): ItemStack
    }

    InventoryMenu "1" --> "*" Page
    Page "1" --> "*" Button

    class InventoryMenuState {
        <<Component>>
        page: usize
        menu: Entity
        inventory: Entity
    }

    InventoryMenuState .. InventoryMenu
    InventoryMenuState .. MenuInventory

    class MenuInventory {
        <<Component>>
        Inventory
    }
    note for MenuInventory "would be struct MenuInventory(Inventory), \nrepresents the Inventory that is used to render\nthe page"

    class Inventory {
        <<Component>>
    }

    MenuInventory --* Inventory

MenuInventory is a wrapper component around Inventory so that queries for Inventory don't conflict with menus.

Unresolved questions:

  • [ ] Tree structure for pages?
  • [ ] Does the notchian client handle stacks of windows? eg. open a window, open another window, press esc to close the top one, does the first window show up again?
  • [ ] What's the best way to let users render menus? Should it be via a trait impl?

What alternative(s) have you considered?

I tried implementing inventory menus without access to valence's internals and was unsuccessful. See playground below

Playground
use valence::client::{default_event_handler, despawn_disconnected_clients};
use valence::inventory::InventorySettings;
use valence::nbt::compound;
use valence::prelude::event::{ClickSlot, PlayerInteractBlock};
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 fn build_app(app: &mut App) {
    app.add_plugin(ServerPlugin::new(()).with_connection_mode(ConnectionMode::Offline))
        .add_startup_system(setup)
        .add_system(default_event_handler.in_schedule(EventLoopSchedule))
        .add_system(init_clients)
        .add_system(despawn_disconnected_clients)
        .add_systems((toggle_gamemode_on_sneak, open_menu).in_schedule(EventLoopSchedule))
        .add_systems((close_menu, render_menus));
}

fn setup(mut commands: Commands, server: Res<Server>, mut inv_settings: ResMut<InventorySettings>) {
    let mut instance = server.new_instance(DimensionId::default());

    for z in -5..5 {
        for x in -5..5 {
            instance.insert_chunk([x, z], Chunk::default());
        }
    }

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

    commands.spawn(instance);

    let menu = InventoryMenu::new(InventoryKind::Generic9x3).with_pages(vec![Page::new()
        .with_buttons(vec![
            Button::new(0, ItemKind::Stone),
            Button::new(10, ItemKind::Dirt).with_name("Totally Real Dirt"),
            Button::new(20, ItemKind::OakLog)
                .with_name("\"Oak\" Log")
                .with_lore("It's actually plastic."),
        ])]);

    commands.spawn(menu);

    inv_settings.enable_item_dupe_check = true;
}

fn init_clients(
    mut clients: Query<(&mut Position, &mut Location), Added<Client>>,
    instances: Query<Entity, With<Instance>>,
) {
    for (mut pos, mut loc) in &mut clients {
        pos.0 = [0.5, SPAWN_Y as f64 + 1.0, 0.5].into();
        loc.0 = instances.single();
    }
}

// Add new systems here!

/// Defines the inventory menu that will control the inventory.
#[derive(Debug, Clone, Component)]
struct InventoryMenu {
    pages: Vec<Page>,
    kind: InventoryKind,
}

impl InventoryMenu {
    pub fn new(kind: InventoryKind) -> Self {
        Self {
            kind,
            pages: Default::default(),
        }
    }

    pub fn with_pages(mut self, pages: Vec<Page>) -> Self {
        self.pages = pages;
        self
    }

    pub fn render(&self, state: &InventoryMenuState) -> Vec<Option<ItemStack>> {
        self.pages[state.page].render(self.kind)
    }
}

/// Attached to the client, indicates which page of the inventory menu is
/// currently open.
#[derive(Debug, Clone, Component)]
struct InventoryMenuState {
    page: usize,
    /// The entity has the inventory menu.
    menu: Entity,
    /// The target inventory to use to render the inventory menu.
    inventory: Entity,
}

impl InventoryMenuState {
    pub fn new(menu: Entity, inventory: Entity) -> Self {
        Self {
            page: 0,
            menu,
            inventory,
        }
    }
}

#[derive(Debug, Clone, Default)]
struct Page {
    buttons: Vec<Button>,
}

impl Page {
    pub fn new() -> Self {
        Self {
            buttons: Vec::new(),
        }
    }

    pub fn with_buttons(mut self, buttons: Vec<Button>) -> Self {
        self.buttons = buttons;
        self
    }

    /// Render the page as a list of slots
    pub fn render(&self, kind: InventoryKind) -> Vec<Option<ItemStack>> {
        let mut slots = vec![None; kind.slot_count() as usize];

        for button in self.buttons.iter() {
            slots[button.position as usize] = Some(button.render());
        }

        slots
    }
}

#[derive(Debug, Clone)]
struct Button {
    /// The slot of the button in the inventory menu.
    position: u16,
    /// The item that will be displayed in the button.
    item: ItemKind,
    /// The text that will be displayed as the item name when the player hovers
    /// over the button.
    name: Option<Text>,
    /// The text that will be displayed as the item lore when the player hovers
    /// over the button. Lore is displayed below the item name, like a
    /// description.
    lore: Option<Text>,
}

impl Button {
    pub fn new(position: u16, item: ItemKind) -> Self {
        Self {
            position,
            item,
            name: None,
            lore: None,
        }
    }

    pub fn with_name(mut self, name: impl Into<Text>) -> Self {
        self.name = Some(name.into());
        self
    }

    pub fn with_lore(mut self, lore: impl Into<Text>) -> Self {
        self.lore = Some(lore.into());
        self
    }

    /// Returns the item stack that will represent this button in the inventory.
    ///
    /// See the "display" entry here: https://minecraft.fandom.com/wiki/Tutorials/Command_NBT_tags
    pub fn render(&self) -> ItemStack {
        // TODO: "Lore" should actually by a list of strings, not a single string.
        ItemStack::new(
            self.item,
            1,
            Some(compound! {
                    "display" => match (self.name.as_ref(), self.lore.as_ref()) {
                        (Some(name), Some(lore)) => compound! {
                            "Name" => name.clone(),
                            "Lore" => lore.clone(),
                        },
                        (Some(name), None) => compound! {
                            "Name" => name.clone(),
                        },
                        (None, Some(lore)) => compound! {
                            "Lore" => lore.clone(),
                        },
                        (None, None) => compound! {
                    }
                }
            }),
        )
    }
}

fn open_menu(
    mut commands: Commands,
    menus: Query<(Entity, &InventoryMenu)>,
    mut events: EventReader<PlayerInteractBlock>,
) {
    let Ok((menu_ent, menu)) = menus.get_single() else {
        return;
    };

    for event in events.iter() {
        if event.position != CHEST_POS.into() {
            continue;
        }
        let menu_inv = Inventory::new(menu.kind);
        let menu_inv_ent = commands.spawn(menu_inv).id();
        let menu_state = InventoryMenuState::new(menu_ent, menu_inv_ent);
        let open_inventory = OpenInventory::new(menu_inv_ent);
        commands
            .entity(event.client)
            .insert((open_inventory, menu_state));
    }
}

fn close_menu(
    mut commands: Commands,
    mut events: RemovedComponents<OpenInventory>,
    clients: Query<(Entity, &InventoryMenuState)>,
) {
    for event in events.iter() {
        let Ok((client, state)) = clients.get(event) else {
            continue;
        };
        commands.entity(state.inventory).despawn();
        commands.entity(client).remove::<InventoryMenuState>();
    }
}

fn render_menus(
    mut clients: Query<(Entity, &InventoryMenuState, &mut CursorItem)>,
    menus: Query<&InventoryMenu>,
    mut inventories: Query<&mut Inventory>,
) {
    // for event in click_slots.iter() {
    //     let Ok((_client, state, mut cursor_item)) = clients.get_mut(event.client)
    // else {         continue;
    //     };
    //     let Ok(menu) = menus.get(state.menu) else {
    //         continue;
    //     };
    //     let Ok(mut inv) = inventories.get_mut(state.inventory) else {
    //         continue;
    //     };
    //     let slots = menu.render(state);
    //     inv.set_all_slots(slots);
    //     cursor_item.set_if_neq(CursorItem(None));
    // }
    for (_, state, mut cursor_item) in clients.iter_mut() {
        let Ok(menu) = menus.get(state.menu) else {
            continue;
        };
        let Ok(mut inv) = inventories.get_mut(state.inventory) else {
            continue;
        };
        if inv.is_added() {
            let slots = menu.render(state);
            inv.set_all_slots(slots);
            cursor_item.set_if_neq(CursorItem(None));
        }
    }
}

Additional context

dyc3 avatar Mar 24 '23 18:03 dyc3

This is a good idea for a feature, but it should be a plugin instead of part of the main project, just due to the project's philosophy of being a "game engine for Minecraft servers".

Given that you were unsuccessful in implementing it without internals, that implies that Valence might lack some of the extensibility features needed to be a proper engine for Minecraft servers.

Maybe, instead of implementing this system directly in Valence, some work should be put into exposing the inventory system more to plugins?

misterghast avatar Mar 24 '23 19:03 misterghast

The way that the inventory system works currently is very tightly coupled with clients. For example, the state id of an inventory needs to be associated with a specific client, it's not transferable to another client. I'm open to suggestions on how to resolve this, and related problems.

I do want inventory menus to be as capable as possible. At minimum, there needs to be some separation from regular inventories and menus such that an end user wouldn't be able to confuse them by accident (either by user error or otherwise).

I think that it's possible that we could refine inventory modifications such that we don't rely on player packets to make modifications. This would imply that we use info from packets to make our own modifications to inventories, rather than reusing data that clients have sent us, eg. Don't apply changes from the slots and carried_item fields, use the other properties to do it instead. This could eliminate the need to validate packets in many scenarios, but it would be a significant undertaking that would need sufficient testing for me to feel confident about it.

We're in no rush to implement this, definitely it's not getting into the 0.2.0 release. I would rather we do it right once than do it wrong 10 times. Now is the time to present any ideas that you think are useful (and please do because I really want this to be good).

dyc3 avatar Mar 25 '23 02:03 dyc3

+1 for doing this in a separate plugin, but I haven't examined your playground yet.

rj00a avatar Mar 25 '23 09:03 rj00a

I kinda want to try to build react style jsx syntax on top of this but this might be way out of scope for valence

tachibanayui avatar Jun 21 '23 07:06 tachibanayui