Skip to content

Commit

Permalink
Fixes instances in inventory module where unnecessery updates happened (
Browse files Browse the repository at this point in the history
#656)

# Objective
This pr attempts to solve some components that got triggered
unnecessarily

can be observed with a simple system like this:
```rust
fn log_inventories(invs: Query<(Entity, &Inventory), Changed<Inventory>>) {
    for inv in invs.iter() {
       println!("inventory updated");
    }
}
```
the above system will print each tick when an inventory is open, this
will result in systems like the one above will trigger unnecessarily

# Solution

To solve this I found the places where we currently "reset" some state
in the inventory module each tick and replaced the modifications with
alternatives that only trigger a modification if the value needed to be
reset (was not in the reset state)
  • Loading branch information
lukashermansson authored Oct 11, 2024
1 parent 282323d commit 70a266a
Showing 1 changed file with 16 additions and 7 deletions.
23 changes: 16 additions & 7 deletions crates/valence_inventory/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ fn update_open_inventories(
for (client_entity, mut client, mut inv_state, cursor_item, mut open_inventory) in &mut clients
{
// Validate that the inventory exists.
let Ok(mut inventory) = inventories.get_mut(open_inventory.entity) else {
let Ok(inventory) = inventories.get_mut(open_inventory.entity) else {
// The inventory no longer exists, so close the inventory.
commands.entity(client_entity).remove::<OpenInventory>();

Expand Down Expand Up @@ -743,17 +743,24 @@ fn update_open_inventories(
}
}
}

open_inventory.client_changed = 0;
inv_state.slots_changed = 0;
inventory.changed = 0;
// Since these happen every gametick we only want to trigger change detection
// if we actually did update these. Otherwise systems that are
// running looking for changes to the `Inventory`,`ClientInventoryState`
// or `OpenInventory` components get unneccerely ran each gametick
open_inventory
.map_unchanged(|f| &mut f.client_changed)
.set_if_neq(0);
inv_state
.map_unchanged(|f| &mut f.slots_changed)
.set_if_neq(0);
inventory.map_unchanged(|f| &mut f.changed).set_if_neq(0);
}
}

fn update_cursor_item(
mut clients: Query<(&mut Client, &mut ClientInventoryState, &CursorItem), Changed<CursorItem>>,
) {
for (mut client, mut inv_state, cursor_item) in &mut clients {
for (mut client, inv_state, cursor_item) in &mut clients {
// The cursor item was not the item the user themselves interacted with
if inv_state.client_updated_cursor_item.as_ref() != Some(&cursor_item.0) {
// Contrary to what you might think, we actually don't want to increment the
Expand All @@ -767,7 +774,9 @@ fn update_cursor_item(
});
}

inv_state.client_updated_cursor_item = None;
inv_state
.map_unchanged(|f| &mut f.client_updated_cursor_item)
.set_if_neq(None);
}
}

Expand Down

0 comments on commit 70a266a

Please sign in to comment.