From d72891105d0f6b5a812cf7d2fdc32684b3e32d45 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 05:21:26 +0000 Subject: [PATCH 1/7] fix(items): keep item GUIDs within Character namespaces --- docs/design/item-guids.md | 52 +++++++ gateway/src/codec/tests.rs | 57 +++++++ module/src/auth.rs | 2 +- module/src/debug/mod.rs | 8 +- module/src/items/guids.rs | 104 +++++++++++++ module/src/items/inventory.rs | 13 +- module/src/items/mod.rs | 2 + module/src/items/ops.rs | 50 +++--- module/src/items/tables.rs | 32 +--- module/src/trade.rs | 24 ++- module/src/transfer/harness.rs | 2 +- module/src/transfer/mod.rs | 17 +- module/src/transfer/transport.rs | 26 ++++ module/src/world.rs | 2 +- module/tests/item_guids.rs | 260 +++++++++++++++++++++++++++++++ 15 files changed, 558 insertions(+), 93 deletions(-) create mode 100644 docs/design/item-guids.md create mode 100644 module/src/items/guids.rs create mode 100644 module/tests/item_guids.rs diff --git a/docs/design/item-guids.md b/docs/design/item-guids.md new file mode 100644 index 000000000..b75daf542 --- /dev/null +++ b/docs/design/item-guids.md @@ -0,0 +1,52 @@ +# Item GUID allocation + +An item keeps its GUID while it moves between inventory slots and during Character Transfer. +Trading creates a new item GUID for the recipient. Mail, Auctions and Buyback retain an item +snapshot and create a new GUID when the recipient takes the item. They do not restore the +original item GUID. + +New item GUIDs retain `HIGHGUID_ITEM = 0x4000` in bits 48 through 63. Bit 47 marks the new +allocation format. Bits 11 through 46 hold the complete Character GUID; bits 0 through 10 +select one of 2,048 item identities. Character GUIDs above 68,719,476,735 receive +`ITEM_GUID_OWNER_OUT_OF_RANGE`, with no truncation. The current billion-sized Character ranges +fit through range 67; range 68 fits only partially. Expanding beyond that requires a reviewed +allocation change before assigning those ranges. + +Allocation selects free identities in this Character's block and checks each candidate against +all local item rows. A batch is selected before any existing stacks change or Trading deletes +outgoing items. A full block returns `ITEM_GUID_EXHAUSTED`, leaving the request unchanged. +Deleted identities can be reused. The limit bounds simultaneously allocated items, not lifetime +grants. Normal inventory has fewer than 2,048 slots. + +Reuse relies on existing ownership rules. Transfer preserves the Character and its items together +under Escrow. Only one Shard may accept inventory changes for that Character. Every operation +that changes an item's Character owner creates a new recipient GUID. A future ownership-changing +path that preserves item GUIDs must also replace this allocation policy. + +## Compatibility and rollout + +No table, column, reducer argument or Transfer row format changes. Existing item rows and saved +Transfer Escrow retain their exact item GUIDs. There is no automatic renumbering or data sweep. +Legacy items may already overlap another Character's namespace. Destination Transfer import +checks arriving item ownership, duplicate GUIDs and local collisions before inserting item rows. +A conflict returns a Refusal; the reducer transaction preserves the destination and the source +Escrow. It never deletes another Character's item or chooses a replacement identity. + +The old allocator normally used only the low 32 bits, but its unchecked increment could escape +that range. Bit 47 alone cannot prove that an existing item was created by the new allocator. +Before deployment, an authorized Operator must inventory item GUIDs and saved Transfer Escrow +across every Shard. Record duplicate GUIDs with different owners, malformed item prefixes, +Character GUIDs above the supported limit and legacy GUIDs with bit 47 set. Resolve those cases +under separate human approval. The allocator's local collision check is not a realm-wide audit. + +Pause World Sessions and all item-granting or ownership-changing activity during the update. +Drain or retain recorded Transfer Escrow, publish the reviewed Module to every Shard, and verify +that all Shards run the same build before resuming activity. An old Module would otherwise mint +from a new item's maximum GUID without respecting the new block boundary. Rolling back has the +same risk and requires the same pause and a reviewed plan for new-format rows. + +Before shipping, obtain human review of this layout and rollout plan. On an authorized isolated +realm, verify existing and new items through grant, split, Trading, Mail, Buyback and round-trip +Transfer, including a conflicting legacy arrival. Check actual 5875 client item rendering and +slot moves. Unit tests and private reducer tests do not establish real-client acceptance or +prove a live realm's existing GUIDs are conflict-free. diff --git a/gateway/src/codec/tests.rs b/gateway/src/codec/tests.rs index fdb56088d..9702ee29e 100644 --- a/gateway/src/codec/tests.rs +++ b/gateway/src/codec/tests.rs @@ -4097,3 +4097,60 @@ fn item_push_and_mail_keep_the_instance_property() { 117 ); } + +#[test] +fn item_guid_namespace_bits_survive_item_and_container_wire_updates() { + for guid in [ + 0x4000_8000_0000_0800, + 0x4000_8165_a0bc_0800, + 0x4000_ffff_ffff_ffff, + ] { + for container_slots in [0, 8] { + let item = ItemInstanceView { + guid, + entry: 25, + owner_guid: 3_000_000_001, + slot: 23, + stack_count: 1, + container_slots, + ..Default::default() + }; + let mut bytes = Vec::new(); + build_item_create_object(&item) + .write_unencrypted_server(&mut bytes) + .unwrap(); + let ServerOpcodeMessage::SMSG_UPDATE_OBJECT(decoded) = + ServerOpcodeMessage::read_unencrypted(&mut bytes.as_slice()).unwrap() + else { + panic!("expected item object update"); + }; + match &decoded.objects[0] { + Object::CreateObject2 { + guid3, + mask2: UpdateMask::Item(fields), + .. + } => { + assert_eq!(guid3.guid(), guid); + assert_eq!(fields.object_guid(), Some(Guid::new(guid))); + } + Object::CreateObject2 { + guid3, + mask2: UpdateMask::Container(fields), + .. + } => { + assert_eq!(guid3.guid(), guid); + assert_eq!(fields.object_guid(), Some(Guid::new(guid))); + } + other => panic!("expected item or container, got {other:?}"), + } + let mut slot_bytes = Vec::new(); + build_inv_slot_values(item.owner_guid, item.slot, guid) + .unwrap() + .write_unencrypted_server(&mut slot_bytes) + .unwrap(); + assert!(slot_bytes + .windows(8) + .any(|bytes| bytes == guid.to_le_bytes())); + } + } +} diff --git a/module/src/auth.rs b/module/src/auth.rs index c4a71165d..6eeee4baa 100644 --- a/module/src/auth.rs +++ b/module/src/auth.rs @@ -1176,7 +1176,7 @@ pub fn create_character( // here: char-select reads ride the gateway's PRIVILEGED coordinator (RLS-bypassed), and any // ZERO/stale owner stamp is corrected by player_login's restamp_owned_data sweep BEFORE the // first player-scoped (RLS) read of these rows can happen. - crate::items::grant_starter_item(ctx, next_guid, owner); + crate::items::grant_starter_item(ctx, next_guid, owner)?; Ok(()) } diff --git a/module/src/debug/mod.rs b/module/src/debug/mod.rs index 6ce399841..03cd8d7b3 100644 --- a/module/src/debug/mod.rs +++ b/module/src/debug/mod.rs @@ -1275,8 +1275,8 @@ pub fn debug_equip_offhand( /// Shared core of the `debug_equip_*` reducers: upsert `character_guid`'s owned item instance in `slot` /// to `item_entry`, stamping the live entity's owner identity (so the per-owner RLS filter still shows -/// it) and seeding durability from the template (0 if the template isn't loaded). The slot-derived guid -/// is unique per slot, so equipping different slots never collides. +/// it) and seeding durability from the template. Find existing gear by slot, since moving it +/// preserves its GUID. fn equip_into( ctx: &ReducerContext, character_guid: u64, @@ -1284,7 +1284,6 @@ fn equip_into( slot: u8, ) -> Result<(), String> { let e = crate::helpers::live_entity(ctx, character_guid)?; - let guid = crate::items::item_guid_for(character_guid, slot); let tmpl = ctx.db.game_item_template().entry().find(item_entry); let random_property_id = tmpl .as_ref() @@ -1299,7 +1298,7 @@ fn equip_into( .map(|t| crate::items::binds_on_grant(t.bonding) || crate::items::binds_on_equip(t.bonding)) .unwrap_or(false); let instances = ctx.db.game_item_instance(); - if let Some(mut inst) = instances.guid().find(guid) { + if let Some(mut inst) = crate::items::item_in_slot(ctx, character_guid, slot) { if inst.entry != item_entry { inst.random_property_id = random_property_id; } @@ -1308,6 +1307,7 @@ fn equip_into( inst.soulbound = inst.soulbound || soulbound; instances.guid().update(inst); } else { + let guid = crate::items::next_item_guid(ctx, character_guid)?; instances.insert(ItemInstance { guid, entry: item_entry, diff --git a/module/src/items/guids.rs b/module/src/items/guids.rs new file mode 100644 index 000000000..86eb4c398 --- /dev/null +++ b/module/src/items/guids.rs @@ -0,0 +1,104 @@ +//! Bounded Character namespaces for newly created item identities. + +use spacetimedb::ReducerContext; + +use super::tables::game_item_instance; + +const ITEM_PREFIX: u64 = 0x4000 << 48; +const FORMAT_BIT: u64 = 1 << 47; +const SERIAL_BITS: u32 = 11; +const CAPACITY: usize = 1 << SERIAL_BITS; +const MAX_OWNER: u64 = (1 << (47 - SERIAL_BITS)) - 1; + +/// Select the complete batch before changing inventory. Trading also calls this before deleting +/// outgoing items, so an incoming item cannot reuse a GUID deleted by the same Trade Commit. +pub(crate) fn allocate_item_guids( + ctx: &ReducerContext, + owner_guid: u64, + count: usize, +) -> Result, String> { + select_item_guids(owner_guid, count, |guid| { + ctx.db.game_item_instance().guid().find(guid).is_some() + }) +} + +pub(crate) fn next_item_guid(ctx: &ReducerContext, owner_guid: u64) -> Result { + Ok(allocate_item_guids(ctx, owner_guid, 1)?[0]) +} + +/// Deleted identities may be reused because ownership changes recreate items in the recipient's +/// namespace. Transfer alone preserves GUIDs and moves the entire Character under Escrow. +fn select_item_guids( + owner_guid: u64, + count: usize, + mut occupied: impl FnMut(u64) -> bool, +) -> Result, String> { + if count == 0 { + return Ok(Vec::new()); + } + if owner_guid > MAX_OWNER { + return Err("ITEM_GUID_OWNER_OUT_OF_RANGE".to_owned()); + } + if count > CAPACITY { + return Err("ITEM_GUID_EXHAUSTED".to_owned()); + } + let base = ITEM_PREFIX | FORMAT_BIT | (owner_guid << SERIAL_BITS); + let mut guids = Vec::with_capacity(count); + for serial in 0..CAPACITY { + let guid = base | serial as u64; + if !occupied(guid) { + guids.push(guid); + if guids.len() == count { + return Ok(guids); + } + } + } + Err("ITEM_GUID_EXHAUSTED".to_owned()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::BTreeSet; + + #[test] + fn full_character_guid_and_item_prefix_survive_allocation() { + let mut issued = BTreeSet::new(); + for owner in [0, 1, 16_777_217, 1_000_000_001, 3_000_000_001, MAX_OWNER] { + let guids = select_item_guids(owner, CAPACITY, |_| false).unwrap(); + for guid in guids { + assert_eq!(guid >> 48, 0x4000); + assert_eq!((guid & ((1 << 47) - 1)) >> 11, owner); + assert!(issued.insert(guid)); + } + } + assert_eq!( + select_item_guids(MAX_OWNER + 1, 1, |_| false).unwrap_err(), + "ITEM_GUID_OWNER_OUT_OF_RANGE" + ); + assert!(select_item_guids(u64::MAX, 1, |_| false).is_err()); + } + + #[test] + fn a_full_block_refuses_without_escaping_to_the_next_character() { + let base = 0x4000_8000_0000_0800; + assert_eq!( + select_item_guids(1, 1, |guid| { + assert!((base..base + 2048).contains(&guid)); + true + }) + .unwrap_err(), + "ITEM_GUID_EXHAUSTED" + ); + assert!(select_item_guids(1, 2049, |_| false).is_err()); + } + + #[test] + fn a_batch_skips_existing_items_and_reuses_only_free_identities() { + let base = 0x4000_8000_0000_0800; + let guids = + select_item_guids(1, 2, |guid| guid != base + 17 && guid != base + 2047).unwrap(); + assert_eq!(guids, [base + 17, base + 2047]); + assert!(select_item_guids(1, 3, |guid| guid != base + 17 && guid != base + 2047).is_err()); + } +} diff --git a/module/src/items/inventory.rs b/module/src/items/inventory.rs index 79e9eb65b..69c721aba 100644 --- a/module/src/items/inventory.rs +++ b/module/src/items/inventory.rs @@ -8,7 +8,7 @@ use spacetimedb::{ReducerContext, Table}; use lyracore_shared::constants::starter_item; use lyracore_shared::item::ItemRefusal; -use super::{refuse, refused}; +use super::{next_item_guid, refuse, refused}; use super::rules::{ binds_on_equip, can_equip_into, eligibility_mask_allows, equip_slot, invtype, @@ -16,8 +16,7 @@ use super::rules::{ resolve_equip_slot, Proficiency, }; use super::tables::{ - game_item_instance, game_item_template, item_in_slot, next_item_guid, slot_occupied, - ItemInstance, + game_item_instance, game_item_template, item_in_slot, slot_occupied, ItemInstance, }; use crate::{game_player_reputation, game_player_skill}; @@ -30,7 +29,7 @@ use crate::{game_player_reputation, game_player_skill}; /// (`valid_split_dest_slot`) — a split can never legitimately land on the body, and unlike /// `apply_item_move` this path runs no equip-validation at all, so admitting 0..=18 here bypassed /// `can_equip_into`/proficiency/required-level/BoE entirely. The new partial-stack row reuses the -/// source's entry / owner / durability and takes a fresh per-slot guid (`item_guid_for`) + the +/// source's entry / owner / durability and takes a fresh Character-namespaced guid and the /// current timestamp. Errors if the source slot is empty, the count is invalid, the destination is /// an equipment slot, or the destination is occupied. Additive — decrements the source row and /// inserts one new item row. [entity] @@ -71,6 +70,8 @@ pub(crate) fn apply_item_split( if slot_occupied(ctx, player_guid, to_slot) { return Err(refuse(ItemRefusal::WrongSlot, "destination slot occupied")); } + let new_guid = + next_item_guid(ctx, player_guid).map_err(|detail| refuse(ItemRefusal::Internal, detail))?; inst.stack_count -= count; let entry = inst.entry; let owner_identity = inst.owner_identity; @@ -78,7 +79,6 @@ pub(crate) fn apply_item_split( let random_property_id = inst.random_property_id; let soulbound = inst.soulbound; // the split half carries the SAME binding state as its source stack instances.guid().update(inst); - let new_guid = next_item_guid(ctx, player_guid, to_slot); instances.insert(ItemInstance { guid: new_guid, entry, @@ -97,8 +97,7 @@ pub(crate) fn apply_item_split( /// Shared move/swap logic for the player + debug paths: move the item in `from_slot` to `to_slot`. /// If `to_slot` holds an item too, the two SWAP slots; if it's empty, the item just moves. The item -/// GUID is its stable identity (only `slot` changes) — `item_guid_for`'s guid↔slot derivation is a -/// grant-time convenience, not an invariant the client relies on after that (it tracks items by guid). +/// GUID remains stable when its inventory slot changes. /// One EXCEPTION to the swap (FEATURE B): if the destination holds the SAME stackable item /// (`dst.entry == src.entry` and the template's `max_stack > 1`), the stacks MERGE instead of swapping /// — `merge_amount` units flow from src into dst (capped by dst's headroom); src is deleted if drained, diff --git a/module/src/items/mod.rs b/module/src/items/mod.rs index f7ec6c818..68ef1d5fb 100644 --- a/module/src/items/mod.rs +++ b/module/src/items/mod.rs @@ -20,6 +20,7 @@ //! reducers (`gw.rs`) are the only surviving callers of these cores. mod economy; +mod guids; mod inventory; mod ops; mod properties; @@ -41,6 +42,7 @@ pub(crate) fn refused(refusal: ItemRefusal) -> String { } pub(crate) use economy::*; // the apply_* cores are pub(crate) (no pub items to re-export) +pub(crate) use guids::*; pub(crate) use inventory::*; // ditto pub(crate) use ops::*; // ditto pub use properties::*; diff --git a/module/src/items/ops.rs b/module/src/items/ops.rs index 8ef9b55eb..e4f90ea1c 100644 --- a/module/src/items/ops.rs +++ b/module/src/items/ops.rs @@ -9,7 +9,7 @@ use spacetimedb::{Identity, ReducerContext, Table}; use lyracore_shared::constants::starter_item; use lyracore_shared::item::ItemRefusal; -use super::{refuse, select_property}; +use super::{allocate_item_guids, next_item_guid, refuse, select_property}; use crate::game_character; // the durable char holds `class` (the live WorldEntity does not) use crate::game_corpse_loot; // the loot.rs accessor trait — re-exported at crate root (`pub use loot::*`) use crate::game_gameobject; @@ -23,8 +23,8 @@ use super::rules::{ template_stat, EquipStat, }; use super::tables::{ - game_item_instance, game_item_template, item_guid_for, item_in_slot, item_is_broken, - next_item_guid, ItemInstance, ItemTemplate, + game_item_instance, game_item_template, item_in_slot, item_is_broken, ItemInstance, + ItemTemplate, }; /// Grant the starter loadout to a character the first time it logs in, idempotently. Called from @@ -35,7 +35,11 @@ use super::tables::{ /// loadout is the EQUIPPED main-hand weapon plus a couple of loose backpack items, proving the /// inventory renders multiple distinct items at distinct slots. Additive: it only ever inserts into /// the two new item tables. [entity] -pub(crate) fn grant_starter_item(ctx: &ReducerContext, owner_guid: u64, owner_identity: Identity) { +pub(crate) fn grant_starter_item( + ctx: &ReducerContext, + owner_guid: u64, + owner_identity: Identity, +) -> Result<(), String> { let instances = ctx.db.game_item_instance(); // Already has items → nothing to do (idempotent across relogs). This one guard gates every grant // below, so the whole loadout is granted at most once per character. @@ -45,19 +49,19 @@ pub(crate) fn grant_starter_item(ctx: &ReducerContext, owner_guid: u64, owner_id .next() .is_some() { - return; + return Ok(()); } // Grant one owned item at `slot`, only if its template is seeded (a missing template is skipped, - // never fatal to login). `item_guid_for(owner_guid, slot)` derives a distinct guid per slot. - let grant_one = |entry: u32, slot: u8, stack: u32| { + // never fatal to login). Allocation still refuses an exhausted Character namespace. + let grant_one = |entry: u32, slot: u8, stack: u32| -> Result<(), String> { let Some(tmpl) = ctx.db.game_item_template().entry().find(entry) else { - return; + return Ok(()); }; let Ok(random_property_id) = select_property(ctx, &tmpl) else { - return; + return Ok(()); }; instances.insert(ItemInstance { - guid: item_guid_for(owner_guid, slot), + guid: next_item_guid(ctx, owner_guid)?, entry: tmpl.entry, owner_identity, owner_guid, @@ -71,6 +75,7 @@ pub(crate) fn grant_starter_item(ctx: &ReducerContext, owner_guid: u64, owner_id soulbound: binds_on_grant(tmpl.bonding), random_property_id, }); + Ok(()) }; // Every character starts with a Hearthstone in the backpack (use it to recall to the bound home). // Granted BEFORE the outfit/fallback branches so both paths get it; HEARTHSTONE_SLOT (38, last @@ -79,7 +84,7 @@ pub(crate) fn grant_starter_item(ctx: &ReducerContext, owner_guid: u64, owner_id starter_item::HEARTHSTONE_ENTRY, starter_item::HEARTHSTONE_SLOT, 1, - ); + )?; // Per-class loadout from CharStartOutfit (game_start_item, importer --dbc): EQUIP each equippable // piece into its resolved slot (so weapons/armor render on the model) and stow the rest in the // backpack — so a Mage spawns with a staff/robe, not the Warrior's sword. Keyed by the character's @@ -128,22 +133,23 @@ pub(crate) fn grant_starter_item(ctx: &ReducerContext, owner_guid: u64, owner_id match resolve_equip_slot(tmpl.inventory_type, false, |s| occupied.contains(&s)) { Some(eq) => { occupied.insert(eq); - grant_one(entry, eq, count); + grant_one(entry, eq, count)?; } None => { - grant_one(entry, backpack, count); + grant_one(entry, backpack, count)?; backpack += 1; } } } - return; + return Ok(()); } // FALLBACK (pre-import / unseeded race_class): the hand-authored Warrior loadout — a weapon EQUIPPED in // the main hand (slot 15) so the client renders it on the model, plus a couple of backpack items. - grant_one(starter_item::ENTRY, starter_item::MAINHAND_SLOT, 1); - grant_one(51, starter_item::BACKPACK_SLOT_0, 1); - grant_one(52, starter_item::BACKPACK_SLOT_0 + 1, 5); + grant_one(starter_item::ENTRY, starter_item::MAINHAND_SLOT, 1)?; + grant_one(51, starter_item::BACKPACK_SLOT_0, 1)?; + grant_one(52, starter_item::BACKPACK_SLOT_0 + 1, 5)?; + Ok(()) } /// Grant `count`× `item_entry` to `player_guid` — the canonical "mint an owned item" core, used by @@ -235,6 +241,7 @@ pub(crate) fn store_item( select_property(ctx, tmpl)? }); } + let guids = allocate_item_guids(ctx, player_guid, new_stacks as usize)?; for mut item in partials { let add = merge_amount(count, item.stack_count, max_stack); if add == 0 { @@ -245,12 +252,12 @@ pub(crate) fn store_item( count -= add; instances.guid().update(item); } - for random_property_id in properties { + for (random_property_id, guid) in properties.into_iter().zip(guids) { // The capacity check counted these same slots, and reducers cannot interleave. let slot = free_slot(ctx, player_guid).expect("preflight reserved a free item slot"); let take = count.min(max_stack); instances.insert(ItemInstance { - guid: next_item_guid(ctx, player_guid, slot), + guid, entry: tmpl.entry, owner_identity, owner_guid: player_guid, @@ -327,7 +334,10 @@ pub(crate) fn store_instance_state( // Trade allocates before deleting either side's outgoing rows: otherwise an emptied inventory // can re-mint a guid deleted earlier in the same transaction and the item relay sees UPDATE // instead of CREATE. Mail can safely allocate at insertion time. - let guid = preallocated_guid.unwrap_or_else(|| next_item_guid(ctx, player_guid, slot)); + let guid = match preallocated_guid { + Some(guid) => guid, + None => next_item_guid(ctx, player_guid)?, + }; ctx.db.game_item_instance().insert(ItemInstance { guid, entry: tmpl.entry, diff --git a/module/src/items/tables.rs b/module/src/items/tables.rs index 3d4a31bb2..315d9094d 100644 --- a/module/src/items/tables.rs +++ b/module/src/items/tables.rs @@ -4,8 +4,6 @@ use spacetimedb::{table, Identity, ReducerContext, Table, Timestamp}; -use lyracore_shared::constants::starter_item; - /// The nine class bits playable by the 1.12 client. Class ids skip 6 and 10, so Druid (11) is bit /// 0x400; the client hides the tooltip "Classes:" line only when every playable bit is set. /// Item-template masks retain unknown bits from imported content; this value exists only to make @@ -241,11 +239,8 @@ crate::character_owned!(delete, fn sweep_delete_game_item_instance(ctx, characte items.guid().delete(r.guid); } }); -// CROSS-DATABASE transport, HOT: worn gear + bags. The item `guid` is PRESERVED, unlike -// every surrogate-PK table here: it is derived from the OWNER guid (`item_guid_for`), so it is -// already namespaced per character and cannot collide with the destination's own items — and it is -// the id the CLIENT knows an item by (equipment slots, loot, trade). Re-minting it would make every -// item look brand new to the arriving client. +// Transfer preserves the item identity the client already knows. New items use a checked +// Character namespace; import also checks collisions because legacy GUIDs can overlap. crate::character_owned!(transfer, fn sweep_transfer_game_item_instance(ctx, character_guid, io) { table = game_item_instance, by = by_owner_guid, @@ -261,29 +256,6 @@ crate::character_owned!(restamp, fn sweep_restamp_game_item_instance(ctx, charac } }); -/// Compose an item GUID: `HIGHGUID_ITEM` (0x4000) in bits 48..63, the low id in the low bits -/// (the vanilla item-guid packing the 5875 client expects). The low id is derived from the owning character -/// so a re-grant after a wipe is deterministic and never collides with a unit/player guid. -pub(crate) fn item_guid_for(owner_guid: u64, slot: u8) -> u64 { - (starter_item::HIGHGUID_ITEM << 48) | ((owner_guid & 0x00FF_FFFF) << 8) | slot as u64 -} - -/// Mint a guid STRICTLY above all of this owner's existing item guids — never reuse -/// `item_guid_for(owner, fallback_slot)`. That slot→guid derivation is only a one-time birth convention -/// on an empty inventory; `move_item` keeps an item's guid fixed while changing its slot, so a -/// slot-derived guid can collide with an already-moved item's PK and panic the insert. `max+1` is -/// unique by construction; fall back to the slot derivation only on an empty inventory. -pub(crate) fn next_item_guid(ctx: &ReducerContext, owner_guid: u64, fallback_slot: u8) -> u64 { - ctx.db - .game_item_instance() - .by_owner_guid() - .filter(&owner_guid) - .map(|i| i.guid) - .max() - .map(|m| m + 1) - .unwrap_or_else(|| item_guid_for(owner_guid, fallback_slot)) -} - /// The owner's item instance occupying inventory `slot`, or `None` if that slot is empty. A point scan /// over the owner's rows (the inventory model has no per-slot index, so this matches each call site's /// existing `by_owner_guid().filter(&owner).find(|i| i.slot == slot)` exactly). diff --git a/module/src/trade.rs b/module/src/trade.rs index e7c562007..e4195487d 100644 --- a/module/src/trade.rs +++ b/module/src/trade.rs @@ -803,20 +803,18 @@ fn run_trade_commit( Ok(()) } Ok(()) => { - // Mint incoming guids from the PRE-delete high-water mark: after the deletes a seat - // can hold zero rows, and `next_item_guid`'s birth-formula fallback could then - // re-mint a guid this very swap just deleted — an insert on a deleted PK is an - // UPDATE to the item relay, which renders the OLD item (the #8 ghost-item gotcha in - // its other form). Pre-delete, max+1 can collide with nothing. - let base_slot = lyracore_shared::constants::starter_item::BACKPACK_SLOT_0; - let mut partner_next = crate::items::next_item_guid(ctx, partner.guid, base_slot); - let mut acceptor_next = crate::items::next_item_guid(ctx, acceptor.guid, base_slot); + // Reserve both batches while outgoing identities still exist. The item Relay must + // observe new identities for the incoming items, even when both inventories empty. + let partner_guids = + crate::items::allocate_item_guids(ctx, partner.guid, acceptor_items.len())?; + let acceptor_guids = + crate::items::allocate_item_guids(ctx, acceptor.guid, partner_items.len())?; // ALL outgoing rows leave first — their slots are the room the verdict counted on. let instances = ctx.db.game_item_instance(); for inst in acceptor_items.iter().chain(partner_items.iter()) { instances.guid().delete(inst.guid); } - for inst in &acceptor_items { + for (inst, guid) in acceptor_items.iter().zip(partner_guids) { let tmpl = ctx .db .game_item_template() @@ -828,12 +826,11 @@ fn run_trade_commit( partner.guid, partner.owner_identity, &tmpl, - Some(partner_next), + Some(guid), &crate::items::ItemSnapshot::from(inst), )?; - partner_next += 1; } - for inst in &partner_items { + for (inst, guid) in partner_items.iter().zip(acceptor_guids) { let tmpl = ctx .db .game_item_template() @@ -845,10 +842,9 @@ fn run_trade_commit( acceptor.guid, acceptor.owner_identity, &tmpl, - Some(acceptor_next), + Some(guid), &crate::items::ItemSnapshot::from(inst), )?; - acceptor_next += 1; } // Gold, both legs through the ONE arithmetic the verdict already approved, applied // to the freshly-read rows (in-transaction they equal the verdict's snapshot; using diff --git a/module/src/transfer/harness.rs b/module/src/transfer/harness.rs index e5b2ff28c..9fec2ffde 100644 --- a/module/src/transfer/harness.rs +++ b/module/src/transfer/harness.rs @@ -1180,7 +1180,7 @@ fn a_stale_copy_from_an_earlier_hop_is_wiped_before_the_arrival_lands() { /// owned row (this guid's OWN item/quest/etc. left behind with no accompanying `game_character` /// row and no `game_transfer_in` witness — the residual state a table-level witness cannot see, /// since it is a fact about a DIFFERENT table) survived untouched and a fresh import landed -/// its rows on top. Item rows are guid-namespaced to the OWNER (`item_guid_for`), so this can +/// its rows on top. The stale rows in this fixture belong to the arriving Character, so this can /// only ever collide with the SAME character's own leftover, never a stranger's — real /// SpacetimeDB would PANIC on the duplicate primary key exactly as `import_character_blob`'s /// crash trace showed on `game_item_instance`. The fake never panics on a duplicate push, so diff --git a/module/src/transfer/mod.rs b/module/src/transfer/mod.rs index 7caeb9b1e..6a1fa0542 100644 --- a/module/src/transfer/mod.rs +++ b/module/src/transfer/mod.rs @@ -1265,21 +1265,8 @@ pub(crate) fn apply_import_blob( c.relocate(decoded.destination()); c.money = decoded.money; let account_id = c.account_id; - // A stale copy from an earlier, half-finished hop in the OTHER direction gets its owned rows - // wiped before the re-insert, so the arriving payload never lands on top of rows that would - // PK-collide (or worse, survive as a second, older loadout). - // - // UNCONDITIONAL: this used to run only `if sink.has_character(guid)`, which wipes - // every owned table when `game_character` itself survived — but says nothing about a table - // that has this guid's rows WITHOUT a `game_character` row to key off. That combination is - // exactly the one no transfer-id witness can see (the witness is a fact about - // `game_transfer_in`, not about `game_item_instance`), and item rows are guid-namespaced to - // the OWNER (`item_guid_for`), so a collision there can only ever be this SAME character's own - // leftover. Sweeping every time makes the destination table set an idempotent function of - // "what belongs to this guid right now" rather than depending on `game_character` also having - // survived intact — cheap when there is nothing to sweep (every delete-by-key on an absent row - // is a no-op) and the only thing standing between a genuinely orphaned row and a hard panic on - // replay. See `orphaned_owned_rows_with_no_character_row_are_wiped_before_a_fresh_import_lands`. + // Remove stale rows for this Character even when its Character row is absent. Item import + // separately checks foreign GUID collisions; legacy packing did not enforce namespaces. sink.cascade_delete_character(guid); sink.insert_character(c); // AC#3: ratchet this database's guid allocator past `guid` NOW, in the same diff --git a/module/src/transfer/transport.rs b/module/src/transfer/transport.rs index f2cd82619..290a08565 100644 --- a/module/src/transfer/transport.rs +++ b/module/src/transfer/transport.rs @@ -11,6 +11,7 @@ use spacetimedb::{log, ReducerContext, SpacetimeType}; use super::TransferOut; +use crate::items::{game_item_instance, ItemInstance}; #[path = "legacy_item_rows.rs"] mod legacy_item_rows; @@ -379,6 +380,31 @@ pub(crate) fn import_rows( payload: &[TableRows], ) -> Result<(), String> { let payload = legacy_item_rows::prepare(payload)?; + if let Some(entry) = payload + .iter() + .find(|entry| entry.table == "game_item_instance") + { + let mut outcome = Ok(()); + let rows = decode_rows::(&entry.rows, &mut outcome); + outcome?; + let mut guids = std::collections::BTreeSet::new(); + for row in rows { + if row.owner_guid != character_guid { + return Err(format!( + "ITEM_GUID_OWNER_MISMATCH: item {} belongs to Character {}", + row.guid, row.owner_guid + )); + } + if !guids.insert(row.guid) + || ctx.db.game_item_instance().guid().find(row.guid).is_some() + { + return Err(format!( + "ITEM_GUID_CONFLICT: item {} for Character {character_guid}", + row.guid + )); + } + } + } import_rows_via( ctx, character_guid, diff --git a/module/src/world.rs b/module/src/world.rs index d435fd972..cc4e609c0 100644 --- a/module/src/world.rs +++ b/module/src/world.rs @@ -1079,7 +1079,7 @@ pub(crate) fn apply_player_login( // this idempotent call (no-op if the character owns ANY item) covers characters created // before that change. It also re-scopes nothing — owner_identity here is the live connection, // matching what the restamp sweep would set anyway. - crate::items::grant_starter_item(ctx, character.guid, owner); + crate::items::grant_starter_item(ctx, character.guid, owner)?; // Weapon skill (rank 14): lazily seed this character's skill lines (Defense + Unarmed + the now-equipped // weapon's line) at the level cap, owner-scoped for the RLS. AFTER grant_starter_item so the starter diff --git a/module/tests/item_guids.rs b/module/tests/item_guids.rs new file mode 100644 index 000000000..4a7ae681f --- /dev/null +++ b/module/tests/item_guids.rs @@ -0,0 +1,260 @@ +mod support; + +use support::Standalone; + +fn fixture(name: &str) -> Standalone { + let mut shard = Standalone::start(name); + shard.publish_module(); + shard.assert_call("claim_operator", &[]); + shard.assert_call("debug_seed_scenario_fixtures", &[]); + shard.assert_call("debug_spawn_player_entity", &["1"]); + shard.assert_call("install_guid_range", &["0"]); + shard.assert_call( + "create_character", + &["1", "\"Other\"", "1", "1", "0", "0", "0", "0", "0", "0"], + ); + shard.assert_sql("UPDATE game_character SET guid = 16777217 WHERE name = 'Other'"); + shard.assert_call("debug_spawn_player_entity", &["16777217"]); + shard.assert_sql("DELETE FROM game_item_instance"); + shard +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn characters_above_bit_24_can_receive_items_together() { + let shard = fixture("item-guid-owners"); + shard.assert_call("debug_grant_item", &["1", "5090050", "1"]); + shard.assert_call("debug_grant_item", &["16777217", "5090050", "1"]); + let rows = shard.query_rows("SELECT guid FROM game_item_instance"); + assert_eq!(rows.len(), 2); + assert_ne!(rows[0]["guid"], rows[1]["guid"]); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn legacy_items_keep_their_identity_and_no_longer_control_new_grants() { + let shard = fixture("item-guid-legacy"); + shard.assert_call("debug_grant_item", &["1", "5090050", "1"]); + let legacy = 0x4000_0000_0000_01ffu64; + shard.assert_sql(&format!( + "UPDATE game_item_instance SET guid = {legacy} WHERE owner_guid = 1" + )); + shard.assert_call("debug_grant_item", &["1", "5090050", "1"]); + let rows = shard.query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1"); + assert_eq!(rows.len(), 2); + assert!(rows.iter().any(|row| row["guid"] == legacy.to_string())); + let fresh: u64 = rows + .iter() + .find(|row| row["guid"] != legacy.to_string()) + .unwrap()["guid"] + .parse() + .unwrap(); + assert_eq!(fresh, 0x4000_8000_0000_0800); + shard.assert_call("debug_move_item", &["1", "23", "25"]); + let moved = + shard.query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1 AND slot = 25"); + assert_eq!(moved[0]["guid"], legacy.to_string()); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn split_and_debug_equipment_allocate_distinct_items_after_slot_moves() { + let shard = fixture("item-guid-split"); + shard.assert_call("debug_grant_item", &["1", "5090052", "5"]); + let before = shard.query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1")[0] + ["guid"] + .clone(); + shard.assert_call("debug_split_item", &["1", "23", "2", "24"]); + let rows = + shard.query_rows("SELECT guid,stack_count FROM game_item_instance WHERE owner_guid = 1"); + assert_eq!(rows.len(), 2); + assert!(rows + .iter() + .any(|row| row["guid"] == before && row["stack_count"] == "3")); + assert!(rows + .iter() + .any(|row| row["guid"] != before && row["stack_count"] == "2")); + shard.assert_call("debug_equip_weapon", &["1", "5090050"]); + shard.assert_call("debug_unequip_item", &["1", "15"]); + shard.assert_call("debug_equip_weapon", &["1", "5090050"]); + let weapons = shard.query_rows( + "SELECT guid,slot FROM game_item_instance WHERE owner_guid = 1 AND entry = 5090050", + ); + assert_eq!(weapons.len(), 2); + assert_ne!(weapons[0]["guid"], weapons[1]["guid"]); + assert!(weapons.iter().any(|row| row["slot"] == "15")); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn trade_remints_for_each_recipient_before_deleting_outgoing_items() { + let shard = fixture("item-guid-trade"); + shard.assert_call("debug_grant_item", &["1", "5090050", "1"]); + shard.assert_call("debug_grant_item", &["16777217", "5090052", "5"]); + let before = shard.query_rows("SELECT guid FROM game_item_instance"); + shard.assert_sql("UPDATE game_world_entity SET x = 100, y = 100, z = 100 WHERE guid = 1"); + shard + .assert_sql("UPDATE game_world_entity SET x = 100, y = 100, z = 100 WHERE guid = 16777217"); + let first = r#"{"guid":1,"ownership":null}"#; + let second = r#"{"guid":16777217,"ownership":null}"#; + shard.assert_call("gw_initiate_trade", &[first, "16777217"]); + shard.assert_call("gw_begin_trade", &[second]); + shard.assert_call("gw_set_trade_item", &[first, "0", "23"]); + shard.assert_call("gw_set_trade_item", &[second, "0", "23"]); + shard.assert_call("gw_accept_trade", &[first]); + shard.assert_call("gw_accept_trade", &[second]); + let after = shard.query_rows("SELECT guid,owner_guid,entry FROM game_item_instance"); + assert_eq!(after.len(), 2); + for row in &after { + assert!(!before.iter().any(|old| old["guid"] == row["guid"])); + let guid: u64 = row["guid"].parse().unwrap(); + let owner: u64 = row["owner_guid"].parse().unwrap(); + assert_eq!((guid & ((1 << 47) - 1)) >> 11, owner); + assert_eq!(row["entry"], if owner == 1 { "5090052" } else { "5090050" }); + } + shard.assert_call("debug_grant_item", &["1", "5090050", "1"]); + let items = shard.query_rows("SELECT guid FROM game_item_instance"); + assert_eq!(items.len(), 3); +} + +fn freeze(shard: &Standalone, id: &str) -> String { + shard.assert_call( + "begin_transfer", + &[ + id, + r#"{"guid":1,"ownership":null}"#, + "0", + "0", + "0", + "0", + "0", + "0", + "true", + ], + ); + let rows = shard.query_rows(&format!( + "SELECT blob FROM game_transfer_out WHERE transfer_id = {id}" + )); + serde_json::to_string(rows[0]["blob"].strip_prefix("0x").unwrap()).unwrap() +} + +fn finish(source: &Standalone, destination: &Standalone, id: &str) { + let operator = r#"{"guid":0,"ownership":null}"#; + source.assert_call("confirm_import", &[id, operator]); + source.assert_call("finish_transfer", &[id, operator]); + destination.assert_call("release_transfer", &[id, operator]); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn transfer_refuses_legacy_collision_then_preserves_both_formats_on_round_trip() { + let source = fixture("item-guid-transfer-source"); + source.assert_call("debug_grant_item", &["1", "5090050", "1"]); + let legacy = 0x4000_0000_0000_0117u64; + source.assert_sql(&format!( + "UPDATE game_item_instance SET guid = {legacy} WHERE owner_guid = 1" + )); + source.assert_call("debug_grant_item", &["1", "5090050", "1"]); + let before = + source.query_rows("SELECT guid,entry,slot FROM game_item_instance WHERE owner_guid = 1"); + let blob = freeze(&source, "5090171"); + let destination = fixture("item-guid-transfer-destination"); + destination.assert_sql("DELETE FROM game_world_entity WHERE guid = 1"); + destination.assert_call("debug_grant_item", &["16777217", "5090050", "1"]); + destination.assert_sql(&format!( + "UPDATE game_item_instance SET guid = {legacy} WHERE owner_guid = 16777217" + )); + let foreign = + destination.query_rows("SELECT * FROM game_item_instance WHERE owner_guid = 16777217"); + let operator = r#"{"guid":0,"ownership":null}"#; + let refusal = destination.call("import_character_blob", &["5090171", &blob, operator]); + assert!(!refusal.status.success()); + assert!(format!( + "{}{}", + String::from_utf8_lossy(&refusal.stdout), + String::from_utf8_lossy(&refusal.stderr) + ) + .contains("ITEM_GUID_CONFLICT")); + assert_eq!( + destination.query_rows("SELECT * FROM game_item_instance WHERE owner_guid = 16777217"), + foreign + ); + assert!(destination + .query_rows("SELECT transfer_id FROM game_transfer_in") + .is_empty()); + assert_eq!( + source.query_rows("SELECT guid,entry,slot FROM game_item_instance WHERE owner_guid = 1"), + before + ); + destination.assert_sql("DELETE FROM game_item_instance WHERE owner_guid = 16777217"); + destination.assert_call("import_character_blob", &["5090171", &blob, operator]); + destination.assert_call("import_character_blob", &["5090171", &blob, operator]); + assert_eq!( + destination + .query_rows("SELECT guid,entry,slot FROM game_item_instance WHERE owner_guid = 1"), + before + ); + finish(&source, &destination, "5090171"); + let back = freeze(&destination, "5090172"); + source.assert_call("import_character_blob", &["5090172", &back, operator]); + finish(&destination, &source, "5090172"); + assert_eq!( + source.query_rows("SELECT guid,entry,slot FROM game_item_instance WHERE owner_guid = 1"), + before + ); + source.assert_call("debug_spawn_player_entity", &["1"]); + source.assert_call("debug_grant_item", &["1", "5090050", "1"]); + assert_eq!( + source + .query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1") + .len(), + 3 + ); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn exhaustion_preserves_partial_stacks_and_freed_identities_can_be_reused() { + let shard = fixture("item-guid-exhaustion"); + // Park rows outside carried inventory to fill the identity block independently of slot capacity. + for _ in 0..128 { + shard.assert_call("debug_grant_item", &["1", "5090050", "16"]); + shard.assert_sql("UPDATE game_item_instance SET slot = 255 WHERE owner_guid = 1"); + } + let base = 0x4000_8000_0000_0800u64; + shard.assert_sql("UPDATE game_item_template SET max_stack = 2 WHERE entry = 5090050"); + shard.assert_sql(&format!( + "UPDATE game_item_instance SET slot = 23 WHERE guid = {base}" + )); + let before = + shard.query_rows("SELECT guid,stack_count FROM game_item_instance WHERE owner_guid = 1"); + assert_eq!(before.len(), 2048); + let refusal = shard.call("debug_grant_item", &["1", "5090050", "2"]); + assert!(!refusal.status.success()); + assert!(format!( + "{}{}", + String::from_utf8_lossy(&refusal.stdout), + String::from_utf8_lossy(&refusal.stderr) + ) + .contains("ITEM_GUID_EXHAUSTED")); + assert_eq!( + shard.query_rows("SELECT guid,stack_count FROM game_item_instance WHERE owner_guid = 1"), + before + ); + let freed = base + 123; + shard.assert_sql(&format!( + "DELETE FROM game_item_instance WHERE guid = {freed}" + )); + shard.assert_call("debug_grant_item", &["1", "5090050", "2"]); + let reused = shard.query_rows(&format!( + "SELECT slot,stack_count FROM game_item_instance WHERE guid = {freed}" + )); + assert_eq!(reused[0]["slot"], "24"); + assert_eq!(reused[0]["stack_count"], "1"); + assert_eq!( + shard.query_rows(&format!( + "SELECT stack_count FROM game_item_instance WHERE guid = {base}" + ))[0]["stack_count"], + "2" + ); +} From b12d39263db5db8ec691e85075fed0123fb63d42 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 05:59:13 +0000 Subject: [PATCH 2/7] fix(items): allocate GUIDs from durable Shard ranges --- CONTEXT.md | 5 + docs/design/item-guids.md | 97 +++--- gateway/src/codec/tests.rs | 10 +- gateway/src/stdb/account_sessions.rs | 2 + gateway/src/stdb/armor.rs | 1 + gateway/src/stdb/reducers.rs | 1 + ...scriptions_character_gone_durable_tests.rs | 1 + gateway/src/world/tests.rs | 1 + module/src/auth.rs | 166 +++++----- module/src/debug/mod.rs | 2 +- module/src/items/guids.rs | 106 +------ module/src/items/inventory.rs | 5 +- module/src/items/ops.rs | 33 +- module/src/items/tables.rs | 4 +- module/src/realm_core.rs | 3 +- module/src/spell/cast/resolve.rs | 42 ++- module/src/spell/cast/targeting.rs | 47 +-- module/src/trade.rs | 7 +- module/src/transfer/mod.rs | 2 +- module/tests/account_ownership.rs | 6 + module/tests/auction_buyout.rs | 1 + module/tests/auction_expiry.rs | 6 +- module/tests/binding_quests.rs | 1 + module/tests/eventai_quest_credit.rs | 1 + module/tests/goober_lock.rs | 1 + module/tests/item_guids.rs | 283 +++++++++++++++--- module/tests/loot_roll_promotion.rs | 1 + module/tests/package_config.rs | 4 + module/tests/package_delta_references.rs | 10 + module/tests/package_runtime_scripts.rs | 4 + module/tests/playerbots_personality.rs | 1 + module/tests/quest_choice_reward.rs | 1 + module/tests/random_properties.rs | 2 + module/tests/random_property_stats.rs | 1 + module/tests/session_expiry.rs | 1 + module/tests/teleport_motion.rs | 1 + module/tests/typed_taxi_contracts.rs | 1 + module/tests/weather.rs | 2 + module/tests/xp_cap.rs | 1 + module/tests/zone_transition.rs | 1 + 40 files changed, 564 insertions(+), 301 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index b09c6043f..5310032b5 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -203,6 +203,11 @@ _Avoid_: invite intent, group request, party order **Home Shard**: The shard that currently holds a Character's row. +**GUID Range**: +A disjoint interval assigned by Realm-core to one Shard. Character creation and item creation +consume the same durable high-water mark in that interval. Issued identities are never reused, +even after deletion or Transfer. Items add their wire type and format bits to the issued value. + **Shard Map**: The rules that assign a map or instance to a shard. diff --git a/docs/design/item-guids.md b/docs/design/item-guids.md index b75daf542..9c92aa38d 100644 --- a/docs/design/item-guids.md +++ b/docs/design/item-guids.md @@ -1,52 +1,61 @@ # Item GUID allocation -An item keeps its GUID while it moves between inventory slots and during Character Transfer. -Trading creates a new item GUID for the recipient. Mail, Auctions and Buyback retain an item -snapshot and create a new GUID when the recipient takes the item. They do not restore the -original item GUID. - -New item GUIDs retain `HIGHGUID_ITEM = 0x4000` in bits 48 through 63. Bit 47 marks the new -allocation format. Bits 11 through 46 hold the complete Character GUID; bits 0 through 10 -select one of 2,048 item identities. Character GUIDs above 68,719,476,735 receive -`ITEM_GUID_OWNER_OUT_OF_RANGE`, with no truncation. The current billion-sized Character ranges -fit through range 67; range 68 fits only partially. Expanding beyond that requires a reviewed -allocation change before assigning those ranges. - -Allocation selects free identities in this Character's block and checks each candidate against -all local item rows. A batch is selected before any existing stacks change or Trading deletes -outgoing items. A full block returns `ITEM_GUID_EXHAUSTED`, leaving the request unchanged. -Deleted identities can be reused. The limit bounds simultaneously allocated items, not lifetime -grants. Normal inventory has fewer than 2,048 slots. - -Reuse relies on existing ownership rules. Transfer preserves the Character and its items together -under Escrow. Only one Shard may accept inventory changes for that Character. Every operation -that changes an item's Character owner creates a new recipient GUID. A future ownership-changing -path that preserves item GUIDs must also replace this allocation policy. +Characters and items consume the same durable high-water mark in a Shard's GUID Range. +Realm-core assigns disjoint billion-value ranges. `install_guid_range` refuses reassignment; +deleting a Character, deleting an item, Transfer and reinstalling the same range never rewind +the mark. A batch must fit below the range's exclusive end before the allocator advances. +The range now budgets Character creation and item creation together. + +New items retain `HIGHGUID_ITEM = 0x4000` in bits 48 through 63. Bit 47 marks this allocation +format. Bits 0 through 46 hold the issued value, so 140,737,488,355,327 is the largest usable +item allocation. Requests above that limit return `ITEM_GUID_EXHAUSTED` before advancing the +mark. Missing ranges return `NO_GUID_RANGE`; a batch crossing a range end returns +`GUID_RANGE_EXHAUSTED`. Items do not truncate or pack their Character owner's GUID. + +Grants reserve complete batches before updating partial stacks or deleting Trading items. +Issued identities are never reused, including items consumed by a quest or craft earlier in +the same request. If a craft cannot grant all products, it restores the original carried +inventory with the original item GUIDs. Profession skill gains and automatic learning wait +until all direct product grants succeed. This restores carried inventory on a refused craft +without minting refund items from an exhausted range. Other effects retain their authored order. +This is not whole-cast rollback: a mixed recipe with triggered casts or other side effects needs +separate acceptance coverage. The committed conjuring fixture has one direct CreateItem effect; +it does not establish that every imported recipe has that shape. + +An item keeps its GUID while moving between inventory slots and during Character Transfer. +Trading creates new recipient items. Mail, Auctions and Buyback store item snapshots and +create new items when the recipient takes them. Preserved identities now rely on disjoint +GUID Ranges and a durable issuer mark, rather than a packed Character namespace. + +Transfer does not move or replace a Shard's allocator. An arriving Character raises the local +mark only when its GUID belongs to the destination's own range. Incoming items need no floor +update because the issuing Shard retains their high-water mark after they leave. ## Compatibility and rollout No table, column, reducer argument or Transfer row format changes. Existing item rows and saved -Transfer Escrow retain their exact item GUIDs. There is no automatic renumbering or data sweep. -Legacy items may already overlap another Character's namespace. Destination Transfer import -checks arriving item ownership, duplicate GUIDs and local collisions before inserting item rows. -A conflict returns a Refusal; the reducer transaction preserves the destination and the source -Escrow. It never deletes another Character's item or chooses a replacement identity. - -The old allocator normally used only the low 32 bits, but its unchecked increment could escape -that range. Bit 47 alone cannot prove that an existing item was created by the new allocator. -Before deployment, an authorized Operator must inventory item GUIDs and saved Transfer Escrow -across every Shard. Record duplicate GUIDs with different owners, malformed item prefixes, -Character GUIDs above the supported limit and legacy GUIDs with bit 47 set. Resolve those cases -under separate human approval. The allocator's local collision check is not a realm-wide audit. +Transfer Escrow keep their exact GUIDs. There is no automatic renumbering or data sweep. +Destination import checks arriving item ownership, duplicate GUIDs and local collisions before +inserting item rows. A conflict returns `ITEM_GUID_CONFLICT`; the reducer transaction preserves +the destination and source Escrow. It never deletes another Character's item to make room. + +The old allocator normally used only the low 32 bits, but unchecked increments could escape +that range. No bit below the item prefix is provably unreachable by the old code. Before +deployment, an authorized Operator must inventory item GUIDs and saved Transfer Escrow across +every Shard. Record duplicate GUIDs with different owners, malformed item prefixes, legacy IDs +with bit 47 set, missing or overlapping GUID Ranges, and allocator marks at or beyond their +range end. Resolve those cases under separate human approval. Local collision checks do not +replace this realm-wide audit. An already escaped Character GUID also needs explicit review. Pause World Sessions and all item-granting or ownership-changing activity during the update. -Drain or retain recorded Transfer Escrow, publish the reviewed Module to every Shard, and verify -that all Shards run the same build before resuming activity. An old Module would otherwise mint -from a new item's maximum GUID without respecting the new block boundary. Rolling back has the -same risk and requires the same pause and a reviewed plan for new-format rows. - -Before shipping, obtain human review of this layout and rollout plan. On an authorized isolated -realm, verify existing and new items through grant, split, Trading, Mail, Buyback and round-trip -Transfer, including a conflicting legacy arrival. Check actual 5875 client item rendering and -slot moves. Unit tests and private reducer tests do not establish real-client acceptance or -prove a live realm's existing GUIDs are conflict-free. +Record outstanding Transfer Escrow. Verify each Shard's permanent range assignment, publish the +reviewed Module to every Shard, and confirm that all Shards run the same build before resuming +activity. An old Module would mint from a new item's maximum GUID without respecting the shared +allocator. Rolling back requires the same pause and a reviewed plan for new-format rows. +Never reset `game_guid_allocator`, reinstall a different range or delete the range registry. + +Before deployment, obtain human review of this allocation and rollout plan. On an authorized +isolated realm, verify existing and new items through grant, split, crafting, Trading, Mail, +Buyback and round-trip Transfer, including a conflicting legacy arrival. Check actual 5875 +client item rendering and slot moves. Unit tests and private reducer tests do not establish +real-client acceptance or prove a live realm's existing GUIDs are conflict-free. diff --git a/gateway/src/codec/tests.rs b/gateway/src/codec/tests.rs index 9702ee29e..d608a1012 100644 --- a/gateway/src/codec/tests.rs +++ b/gateway/src/codec/tests.rs @@ -4099,10 +4099,10 @@ fn item_push_and_mail_keep_the_instance_property() { } #[test] -fn item_guid_namespace_bits_survive_item_and_container_wire_updates() { +fn item_guid_allocation_bits_survive_item_and_container_wire_updates() { for guid in [ 0x4000_8000_0000_0800, - 0x4000_8165_a0bc_0800, + 0x4000_8000_3b9a_ca01, 0x4000_ffff_ffff_ffff, ] { for container_slots in [0, 8] { @@ -4127,17 +4127,19 @@ fn item_guid_namespace_bits_survive_item_and_container_wire_updates() { match &decoded.objects[0] { Object::CreateObject2 { guid3, + object_type: ObjectType::Item, mask2: UpdateMask::Item(fields), .. - } => { + } if container_slots == 0 => { assert_eq!(guid3.guid(), guid); assert_eq!(fields.object_guid(), Some(Guid::new(guid))); } Object::CreateObject2 { guid3, + object_type: ObjectType::Container, mask2: UpdateMask::Container(fields), .. - } => { + } if container_slots == 8 => { assert_eq!(guid3.guid(), guid); assert_eq!(fields.object_guid(), Some(Guid::new(guid))); } diff --git a/gateway/src/stdb/account_sessions.rs b/gateway/src/stdb/account_sessions.rs index c133daecc..f982fb538 100644 --- a/gateway/src/stdb/account_sessions.rs +++ b/gateway/src/stdb/account_sessions.rs @@ -290,6 +290,7 @@ mod tests { let mut fixture = Standalone::start("account-transfer-destination"); fixture.publish_module(); fixture.assert_call("claim_operator", &[]); + fixture.assert_call("install_guid_range", &["1000000000"]); let destination = runtime.block_on(Coordinator::connect(&GatewayConfig { stdb_uri: fixture.server().into(), module_name: fixture.shard_name().into(), @@ -415,6 +416,7 @@ mod tests { let mut fixture = Standalone::start("account-ownership"); fixture.publish_module(); fixture.assert_call("claim_operator", &[]); + fixture.assert_call("install_guid_range", &["0"]); fixture.assert_call("gw_heartbeat", &[]); let cfg = GatewayConfig { logon_bind: "127.0.0.1:0".into(), diff --git a/gateway/src/stdb/armor.rs b/gateway/src/stdb/armor.rs index e130537d2..47c408d37 100644 --- a/gateway/src/stdb/armor.rs +++ b/gateway/src/stdb/armor.rs @@ -228,6 +228,7 @@ mod tests { let mut shard = Standalone::start("property-armor"); shard.publish_module(); shard.assert_call("claim_operator", &[]); + shard.assert_call("install_guid_range", &["0"]); shard.assert_call("debug_seed_scenario_fixtures", &[]); shard.assert_call("debug_spawn_player_entity", &["1"]); shard.assert_sql("DELETE FROM game_item_instance WHERE owner_guid = 1"); diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index fdb3260a0..b6c69c866 100644 --- a/gateway/src/stdb/reducers.rs +++ b/gateway/src/stdb/reducers.rs @@ -3652,6 +3652,7 @@ mod auction_reducer_tests { let mut standalone = Standalone::start("auction-refusal"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); let cfg = GatewayConfig { logon_bind: "127.0.0.1:0".into(), world_bind: "127.0.0.1:0".into(), diff --git a/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs b/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs index d8ae06e29..7c3577667 100644 --- a/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs +++ b/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs @@ -128,6 +128,7 @@ fn another_gateway_waits_for_the_transfer_then_cleans_the_deleted_character() { standalone.assert_output_success(&output, &format!("publish {database}")); } standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["2000000000"]); for database in [INSTANCES, OTHER, REALM] { let output = cli.call(standalone.server(), database, "claim_operator", &[]); standalone.assert_output_success(&output, &format!("claim {database} operator")); diff --git a/gateway/src/world/tests.rs b/gateway/src/world/tests.rs index 004f13c47..a6d3963dd 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -10091,6 +10091,7 @@ fn competing_world_sessions_close_the_old_socket_without_removing_the_winner() { let mut fixture = Standalone::start("account-world-sessions"); fixture.publish_module(); fixture.assert_call("claim_operator", &[]); + fixture.assert_call("install_guid_range", &["0"]); fixture.assert_call("gw_heartbeat", &[]); let cfg = GatewayConfig { logon_bind: "127.0.0.1:0".into(), diff --git a/module/src/auth.rs b/module/src/auth.rs index 6eeee4baa..5ec4b7092 100644 --- a/module/src/auth.rs +++ b/module/src/auth.rs @@ -510,26 +510,9 @@ mod session_expiry_tests { // Realm-safe guid allocation // =========================================================================================== -/// Durable high-water mark for `game_character.guid`. Singleton row (`id` is always 0). Private — -/// nothing outside this crate reads it. -/// -/// Why this exists: the old allocator was a per-transaction scan of THIS database's own -/// `game_character`/`game_item_instance` rows, which only ever sees what's still HERE. The -/// escrowed transfer's `finish_transfer` deletes the departed character's row on the source -/// database (and, via the `character_owned!` sweep, its item rows too) — so the instant a -/// character transfers away, its guid looked free again on the database it left, and the next -/// `create_character` there could re-issue it while the transferred character is still alive on -/// the destination. Two characters, one guid, two databases. -/// -/// This mark is monotonic: `next_character_guid` bumps it by one on every local create, and -/// `crate::transfer::apply_import_blob` bumps it (via `bump_guid_high_water`) to at least the -/// guid of every character this database RECEIVES from another one. Neither path ever lowers it, -/// so a guid this database has ever handed out — locally or by import — can never be handed out -/// here again, even after the character that held it is long deleted. -/// -/// `create_character` is unrouted (§ premise check): the gateway always calls it -/// against the account's home/default database, never a routed shard, so a per-database mark is -/// sufficient — no cross-database plumbing on the creation path itself. +/// Shared durable high-water mark for Character and item creation. Deletion and Transfer never +/// lower it. Transfer advances it for an arriving Character only inside this Shard's own range. +/// A scan of surviving rows cannot replace it, because issued identities may now live elsewhere. #[table(accessor = game_guid_allocator)] pub struct GuidAllocator { #[primary_key] @@ -620,14 +603,63 @@ pub(crate) fn next_character_guid(ctx: &ReducerContext) -> u64 { next } -/// Ratchet the mark up to at least `guid`. Two call sites: `transfer::apply_import_blob` -/// right after it materialises an imported character (AC#3 — so THIS database can never later -/// re-issue a guid it just received from another one), and `world::cascade_delete_character` right -/// before it removes a `game_character` row on ANY delete path (defect 1 — so a database that has -/// never yet seeded its allocator can't have a guid freed out from under a scan that would otherwise -/// miss it). A no-op once the mark is already seeded and ahead. Pinned by -/// [`guid_allocator_tests::bump_guid_high_water_routes_through_ratchet_high_water`]. +/// Reserve a batch from this Shard's GUID Range. Items and Characters consume the same durable +/// high-water mark; deletion and Transfer never return issued identities to the range. +pub(crate) fn reserve_guids( + ctx: &ReducerContext, + count: u64, + max_guid: u64, +) -> Result, String> { + let existing = read_high_water(ctx); + let mark = existing + .as_ref() + .map(|row| row.high_water) + .unwrap_or_else(|| legacy_guid_seed_now(ctx)); + let range = ctx + .db + .game_guid_range() + .id() + .find(0) + .map(|row| (row.base, row.size)); + let last = guid_batch_end(range, mark, count, max_guid)?; + write_high_water(ctx, existing, last); + Ok(mark + 1..=last) +} + +fn guid_batch_end( + range: Option<(u64, u64)>, + mark: u64, + count: u64, + max_guid: u64, +) -> Result { + let (base, size) = range.ok_or_else(|| "NO_GUID_RANGE".to_owned())?; + let end = base + .checked_add(size) + .ok_or_else(|| "GUID_RANGE_EXHAUSTED".to_owned())?; + let last = mark + .checked_add(count) + .ok_or_else(|| "GUID_RANGE_EXHAUSTED".to_owned())?; + if count == 0 || mark < base || last >= end { + return Err("GUID_RANGE_EXHAUSTED".to_owned()); + } + if last > max_guid { + return Err("ITEM_GUID_EXHAUSTED".to_owned()); + } + Ok(last) +} + +/// Preserve a local Character's issued GUID before import or deletion. Foreign Characters must +/// never advance this Shard's mark, including the cleanup that precedes a Transfer import. pub(crate) fn bump_guid_high_water(ctx: &ReducerContext, guid: u64) { + let range = ctx + .db + .game_guid_range() + .id() + .find(0) + .map(|row| (row.base, row.size)); + if !in_guid_range(range, guid) { + return; + } let existing = read_high_water(ctx); let seed = existing .as_ref() @@ -640,37 +672,8 @@ pub(crate) fn bump_guid_high_water(ctx: &ReducerContext, guid: u64) { write_high_water(ctx, existing, mark); } -/// **The guid range THIS database was assigned, and its licence to mint.** -/// -/// `game_character.guid` is a per-database `#[auto_inc]`-style mark, so two databases that both -/// MINT characters mint colliding guids — and `transfer_id` **is** the character guid, so a -/// collision means two characters sharing an escrow ledger row. It was demonstrated live: guid 59 -/// was one character on `lyracore` and a different one on `lyracore-world-1`, and routing -/// sent the second player to the first's shard. The receiving half already exists -/// (`apply_import_blob` and `cascade_delete_character` ratchet the mark), so a shard can never -/// re-issue a guid it was *given*; this table is the other half: a shard that mints its own starts -/// from a floor nobody else will reach. -/// -/// Convention (`docs/design/`): shard *n* gets floor `n * 1_000_000_000`. Decimal and small on -/// purpose — it stays readable in logs and SQL, leaves a billion characters per shard, and stays far -/// below both `2^53` (above which `spacetime call` mangles u64 arguments — danger-zones) and the -/// high-bit type markers real entity guids carry. -/// -/// the first cut (since deleted) gave each database its floor by hand through a -/// standalone reducer that only ratcheted `game_guid_allocator.high_water` — a collision became -/// *impossible*, but the guarantee rested on an operator remembering to run it on every new shard; -/// a shard provisioned without it minted from zero into `lyracore`'s range, silently at creation, -/// and surfaced later as a mis-routed login or two characters sharing an escrow row (`transfer_id` -/// IS the character guid). -/// -/// So the floor is no longer something a database can lack: the presence of this row is what -/// permits [`next_character_guid`] to run at all, and only [`install_guid_range`] writes it, from a -/// range realm-core assigned. A shard that never claimed one refuses to create characters — loudly, -/// at the point of creation — rather than minting guids that belong to someone else. -/// -/// A range is never *reassigned*: guids already minted from it are referenced by rows that outlive -/// the character (escrow ledgers, party rosters, the shard index, mail), so moving a shard's range -/// would strand them. +/// The permanent GUID Range assigned by Realm-core. Character and item creation both require it. +/// Reassignment would let the Shard reuse identities still held in other Shards or Escrow. #[table(accessor = game_guid_range)] pub struct GuidRange { #[primary_key] @@ -679,16 +682,10 @@ pub struct GuidRange { pub size: u64, } -/// May this database mint a character guid? Pure — the whole rule, so it is testable without a -/// `ReducerContext` and so the refusal message has one source. +/// May this Shard mint one more GUID? The next value must remain below the exclusive range end. +/// Pure, so both allocation paths use the same range boundary. pub(crate) fn may_mint(range: Option<(u64, u64)>, mark: u64) -> Result<(), String> { - let Some((base, size)) = range else { - return Err("NO_GUID_RANGE".to_string()); - }; - if mark >= base.saturating_add(size) { - return Err("GUID_RANGE_EXHAUSTED".to_string()); - } - Ok(()) + guid_batch_end(range, mark, 1, u64::MAX).map(|_| ()) } /// [`may_mint`]'s ctx glue: read this database's range + mark and apply the rule. Called by @@ -800,6 +797,27 @@ mod guid_allocator_tests { ); } + #[test] + fn guid_batches_share_the_character_boundary_and_refuse_overflow() { + use super::guid_batch_end; + assert_eq!(guid_batch_end(Some((100, 10)), 106, 3, u64::MAX), Ok(109)); + assert_eq!( + guid_batch_end(Some((100, 10)), 109, 1, u64::MAX).unwrap_err(), + "GUID_RANGE_EXHAUSTED" + ); + assert!(guid_batch_end(Some((100, 10)), 106, 4, u64::MAX).is_err()); + assert!(guid_batch_end(Some((u64::MAX - 1, 2)), u64::MAX - 1, 1, u64::MAX).is_err()); + assert!(guid_batch_end(Some((100, 10)), 99, 1, u64::MAX).is_err()); + assert_eq!( + guid_batch_end(Some((100, 10)), 106, 3, 108).unwrap_err(), + "ITEM_GUID_EXHAUSTED" + ); + assert_eq!( + guid_batch_end(None, 0, 1, u64::MAX).unwrap_err(), + "NO_GUID_RANGE" + ); + } + /// The floor above is monotonic but *optional* — a shard nobody ran it on mints /// from zero. This is the rule that makes it non-optional, and the two refusals are the whole /// point: no range at all, and a mark that has escaped the range it was assigned. @@ -816,7 +834,8 @@ mod guid_allocator_tests { // A licensed shard mints anywhere inside its range, including at the very base. assert!(may_mint(Some((1_000_000_000, 1_000_000_000)), 1_000_000_000).is_ok()); assert!(may_mint(Some((1_000_000_000, 1_000_000_000)), 1_999_999_998).is_ok()); - // …and stops at the boundary rather than walking into the next shard's guids. + assert!(may_mint(Some((1_000_000_000, 1_000_000_000)), 1_999_999_999).is_err()); + // The next GUID must stay below the exclusive end. assert_eq!( may_mint(Some((1_000_000_000, 1_000_000_000)), 2_000_000_000).unwrap_err(), "GUID_RANGE_EXHAUSTED" @@ -998,6 +1017,15 @@ mod guid_allocator_tests { ); let want = norm( "{ + let range = ctx + .db + .game_guid_range() + .id() + .find(0) + .map(|row| (row.base, row.size)); + if !in_guid_range(range, guid) { + return; + } let existing = read_high_water(ctx); let seed = existing .as_ref() @@ -1014,7 +1042,7 @@ mod guid_allocator_tests { got, want, "bump_guid_high_water's body no longer matches the pinned shape — it must seed from \ the durable mark first, floor it at `guid` via the tested `ratchet_high_water`, and \ - persist the result (skipping the write ONLY on the already-seeded, already-ahead \ + persist the result for local GUIDs (skipping the write on the already-seeded, already-ahead \ no-op). Got:\n{got}" ); } diff --git a/module/src/debug/mod.rs b/module/src/debug/mod.rs index 03cd8d7b3..56eea5680 100644 --- a/module/src/debug/mod.rs +++ b/module/src/debug/mod.rs @@ -1307,7 +1307,7 @@ fn equip_into( inst.soulbound = inst.soulbound || soulbound; instances.guid().update(inst); } else { - let guid = crate::items::next_item_guid(ctx, character_guid)?; + let guid = crate::items::next_item_guid(ctx)?; instances.insert(ItemInstance { guid, entry: item_entry, diff --git a/module/src/items/guids.rs b/module/src/items/guids.rs index 86eb4c398..4ce17f836 100644 --- a/module/src/items/guids.rs +++ b/module/src/items/guids.rs @@ -1,104 +1,30 @@ -//! Bounded Character namespaces for newly created item identities. +//! Item identities issued from the Shard's durable GUID Range. use spacetimedb::ReducerContext; use super::tables::game_item_instance; -const ITEM_PREFIX: u64 = 0x4000 << 48; +const ITEM_PREFIX: u64 = lyracore_shared::constants::starter_item::HIGHGUID_ITEM << 48; const FORMAT_BIT: u64 = 1 << 47; -const SERIAL_BITS: u32 = 11; -const CAPACITY: usize = 1 << SERIAL_BITS; -const MAX_OWNER: u64 = (1 << (47 - SERIAL_BITS)) - 1; +const MAX_LOW_ID: u64 = FORMAT_BIT - 1; -/// Select the complete batch before changing inventory. Trading also calls this before deleting -/// outgoing items, so an incoming item cannot reuse a GUID deleted by the same Trade Commit. -pub(crate) fn allocate_item_guids( - ctx: &ReducerContext, - owner_guid: u64, - count: usize, -) -> Result, String> { - select_item_guids(owner_guid, count, |guid| { - ctx.db.game_item_instance().guid().find(guid).is_some() - }) -} - -pub(crate) fn next_item_guid(ctx: &ReducerContext, owner_guid: u64) -> Result { - Ok(allocate_item_guids(ctx, owner_guid, 1)?[0]) -} - -/// Deleted identities may be reused because ownership changes recreate items in the recipient's -/// namespace. Transfer alone preserves GUIDs and moves the entire Character under Escrow. -fn select_item_guids( - owner_guid: u64, - count: usize, - mut occupied: impl FnMut(u64) -> bool, -) -> Result, String> { +/// Reserve the complete batch before changing inventory. The shared high-water mark never +/// rewinds, so crafting and Trading cannot reuse identities deleted in the same transaction. +pub(crate) fn allocate_item_guids(ctx: &ReducerContext, count: usize) -> Result, String> { if count == 0 { return Ok(Vec::new()); } - if owner_guid > MAX_OWNER { - return Err("ITEM_GUID_OWNER_OUT_OF_RANGE".to_owned()); - } - if count > CAPACITY { - return Err("ITEM_GUID_EXHAUSTED".to_owned()); - } - let base = ITEM_PREFIX | FORMAT_BIT | (owner_guid << SERIAL_BITS); - let mut guids = Vec::with_capacity(count); - for serial in 0..CAPACITY { - let guid = base | serial as u64; - if !occupied(guid) { - guids.push(guid); - if guids.len() == count { - return Ok(guids); - } - } + let ids = crate::auth::reserve_guids(ctx, count as u64, MAX_LOW_ID)?; + let guids: Vec = ids.map(|id| ITEM_PREFIX | FORMAT_BIT | id).collect(); + if let Some(guid) = guids + .iter() + .find(|guid| ctx.db.game_item_instance().guid().find(**guid).is_some()) + { + return Err(format!("ITEM_GUID_CONFLICT: item {guid}")); } - Err("ITEM_GUID_EXHAUSTED".to_owned()) + Ok(guids) } -#[cfg(test)] -mod tests { - use super::*; - use std::collections::BTreeSet; - - #[test] - fn full_character_guid_and_item_prefix_survive_allocation() { - let mut issued = BTreeSet::new(); - for owner in [0, 1, 16_777_217, 1_000_000_001, 3_000_000_001, MAX_OWNER] { - let guids = select_item_guids(owner, CAPACITY, |_| false).unwrap(); - for guid in guids { - assert_eq!(guid >> 48, 0x4000); - assert_eq!((guid & ((1 << 47) - 1)) >> 11, owner); - assert!(issued.insert(guid)); - } - } - assert_eq!( - select_item_guids(MAX_OWNER + 1, 1, |_| false).unwrap_err(), - "ITEM_GUID_OWNER_OUT_OF_RANGE" - ); - assert!(select_item_guids(u64::MAX, 1, |_| false).is_err()); - } - - #[test] - fn a_full_block_refuses_without_escaping_to_the_next_character() { - let base = 0x4000_8000_0000_0800; - assert_eq!( - select_item_guids(1, 1, |guid| { - assert!((base..base + 2048).contains(&guid)); - true - }) - .unwrap_err(), - "ITEM_GUID_EXHAUSTED" - ); - assert!(select_item_guids(1, 2049, |_| false).is_err()); - } - - #[test] - fn a_batch_skips_existing_items_and_reuses_only_free_identities() { - let base = 0x4000_8000_0000_0800; - let guids = - select_item_guids(1, 2, |guid| guid != base + 17 && guid != base + 2047).unwrap(); - assert_eq!(guids, [base + 17, base + 2047]); - assert!(select_item_guids(1, 3, |guid| guid != base + 17 && guid != base + 2047).is_err()); - } +pub(crate) fn next_item_guid(ctx: &ReducerContext) -> Result { + Ok(allocate_item_guids(ctx, 1)?[0]) } diff --git a/module/src/items/inventory.rs b/module/src/items/inventory.rs index 69c721aba..ec7141f24 100644 --- a/module/src/items/inventory.rs +++ b/module/src/items/inventory.rs @@ -29,7 +29,7 @@ use crate::{game_player_reputation, game_player_skill}; /// (`valid_split_dest_slot`) — a split can never legitimately land on the body, and unlike /// `apply_item_move` this path runs no equip-validation at all, so admitting 0..=18 here bypassed /// `can_equip_into`/proficiency/required-level/BoE entirely. The new partial-stack row reuses the -/// source's entry / owner / durability and takes a fresh Character-namespaced guid and the +/// source's entry / owner / durability and takes a fresh GUID and the /// current timestamp. Errors if the source slot is empty, the count is invalid, the destination is /// an equipment slot, or the destination is occupied. Additive — decrements the source row and /// inserts one new item row. [entity] @@ -70,8 +70,7 @@ pub(crate) fn apply_item_split( if slot_occupied(ctx, player_guid, to_slot) { return Err(refuse(ItemRefusal::WrongSlot, "destination slot occupied")); } - let new_guid = - next_item_guid(ctx, player_guid).map_err(|detail| refuse(ItemRefusal::Internal, detail))?; + let new_guid = next_item_guid(ctx).map_err(|detail| refuse(ItemRefusal::Internal, detail))?; inst.stack_count -= count; let entry = inst.entry; let owner_identity = inst.owner_identity; diff --git a/module/src/items/ops.rs b/module/src/items/ops.rs index e4f90ea1c..31e97f8b7 100644 --- a/module/src/items/ops.rs +++ b/module/src/items/ops.rs @@ -52,7 +52,7 @@ pub(crate) fn grant_starter_item( return Ok(()); } // Grant one owned item at `slot`, only if its template is seeded (a missing template is skipped, - // never fatal to login). Allocation still refuses an exhausted Character namespace. + // never fatal to login). Allocation still refuses an exhausted GUID Range. let grant_one = |entry: u32, slot: u8, stack: u32| -> Result<(), String> { let Some(tmpl) = ctx.db.game_item_template().entry().find(entry) else { return Ok(()); @@ -61,7 +61,7 @@ pub(crate) fn grant_starter_item( return Ok(()); }; instances.insert(ItemInstance { - guid: next_item_guid(ctx, owner_guid)?, + guid: next_item_guid(ctx)?, entry: tmpl.entry, owner_identity, owner_guid, @@ -241,7 +241,7 @@ pub(crate) fn store_item( select_property(ctx, tmpl)? }); } - let guids = allocate_item_guids(ctx, player_guid, new_stacks as usize)?; + let guids = allocate_item_guids(ctx, new_stacks as usize)?; for mut item in partials { let add = merge_amount(count, item.stack_count, max_stack); if add == 0 { @@ -336,7 +336,7 @@ pub(crate) fn store_instance_state( // instead of CREATE. Mail can safely allocate at insertion time. let guid = match preallocated_guid { Some(guid) => guid, - None => next_item_guid(ctx, player_guid)?, + None => next_item_guid(ctx)?, }; ctx.db.game_item_instance().insert(ItemInstance { guid, @@ -407,6 +407,31 @@ pub(crate) fn remove_items( Ok(()) } +pub(crate) fn carried_items(ctx: &ReducerContext, owner_guid: u64) -> Vec { + ctx.db + .game_item_instance() + .by_owner_guid() + .filter(&owner_guid) + .filter(|item| is_carried_slot(item.slot)) + .collect() +} + +/// Restore a failed craft before its item changes become visible to the Gateway. This also +/// removes products from earlier effects in the same craft, without minting refund identities. +pub(crate) fn restore_carried_items( + ctx: &ReducerContext, + owner_guid: u64, + items: Vec, +) { + let instances = ctx.db.game_item_instance(); + for item in carried_items(ctx, owner_guid) { + instances.guid().delete(item.guid); + } + for item in items { + instances.insert(item); + } +} + /// "Recently Bandaged" debuff spell id (vanilla 11196) — already seeded (a 60s `A_FLAG` marker aura, no /// tick). The bandage's eff1 `E_TRIGGER` applies it, and `apply_item_use` gates on `has_aura(.., this)` /// BEFORE consuming/casting so a player can't re-bandage while it is live (vanilla's anti-spam window). diff --git a/module/src/items/tables.rs b/module/src/items/tables.rs index 315d9094d..0b2e56348 100644 --- a/module/src/items/tables.rs +++ b/module/src/items/tables.rs @@ -239,8 +239,8 @@ crate::character_owned!(delete, fn sweep_delete_game_item_instance(ctx, characte items.guid().delete(r.guid); } }); -// Transfer preserves the item identity the client already knows. New items use a checked -// Character namespace; import also checks collisions because legacy GUIDs can overlap. +// Transfer preserves the item identity the client already knows. New items consume the Shard's +// GUID Range; import also checks collisions because legacy GUIDs can overlap. crate::character_owned!(transfer, fn sweep_transfer_game_item_instance(ctx, character_guid, io) { table = game_item_instance, by = by_owner_guid, diff --git a/module/src/realm_core.rs b/module/src/realm_core.rs index 7edc3cd0c..76ae468b7 100644 --- a/module/src/realm_core.rs +++ b/module/src/realm_core.rs @@ -126,7 +126,8 @@ pub fn set_character_shard( /// makes the hand-applied floors (core 0, world-1 1e9, instances 2e9, realm-core 3e9) slots /// 0/1/2/3 exactly — so the live databases migrate into this registry by being *recorded*, with no /// guid changing. Decimal and small on purpose (`GuidRange`'s doc, `auth.rs`): readable in logs and SQL, -/// a billion characters per shard, and far below `2^53`, above which `spacetime call` mangles a u64 +/// a billion issued identities per Shard shared by Characters and items. Current assignments are +/// far below `2^53`, above which `spacetime call` mangles a u64 /// argument (danger-zones). pub const GUID_RANGE_SIZE: u64 = 1_000_000_000; diff --git a/module/src/spell/cast/resolve.rs b/module/src/spell/cast/resolve.rs index 5bb2c22b3..eed992b54 100644 --- a/module/src/spell/cast/resolve.rs +++ b/module/src/spell/cast/resolve.rs @@ -128,17 +128,18 @@ pub(crate) fn resolve_cast_at( break_stealth(ctx, caster_guid); } - // CRAFT reagent gate (282, data-driven) — the profession-loop's reagent cost, co-located with the - // money/power gates above and BEFORE the E_CREATE_ITEM product grant. A recipe = a spell with a - // CREATE-ITEM effect AND imported reagents; consume EVERY reagent now (multi-reagent). `remove_items` - // returns Err (rolling the whole tx back) when a reagent is short, so a craft you can't afford fails - // atomically — no product, no skill, no power spent. Scoped to create-item spells so a reagent- - // consuming BUFF (Arcane Intellect → Arcane Powder, no CreateItem effect) is untouched (no regression); - // a conjure (CreateItem, no reagents) has an empty list → no-op. - if effects.iter().any(|e| e.kind == E_CREATE_ITEM) { - for (item, count) in recipe_reagents(ctx, spell_id) { - crate::items::remove_items(ctx, caster_guid, item, count)?; - } + // Keep the original identities so a failed product grant can restore reagents even when the + // GUID Range is exhausted. Skills are awarded only after every product grant succeeds. + let creates_items = effects.iter().any(|effect| effect.kind == E_CREATE_ITEM); + let reagents = if creates_items { + recipe_reagents(ctx, spell_id) + } else { + Vec::new() + }; + let inventory_before_craft = + creates_items.then(|| crate::items::carried_items(ctx, caster_guid)); + for (item, count) in reagents { + crate::items::remove_items(ctx, caster_guid, item, count)?; } // The effect loop + the ONE cast visual — shared verbatim with the Triggered Cast (`cast_triggered`), @@ -155,6 +156,7 @@ pub(crate) fn resolve_cast_at( client_initiated, }, dest, + inventory_before_craft, ); // AGGRO-ON-HOSTILE-CAST: a player casting an ENEMY-targeting spell at a creature pulls it into combat at @@ -322,6 +324,7 @@ fn run_spell_effects( level: u8, origin: CastOrigin, dest: Option<(f32, f32, f32)>, + inventory_before_craft: Option>, ) { let mut hit_targets = Vec::new(); let mut seen_hit_targets = std::collections::HashSet::new(); @@ -331,6 +334,8 @@ fn run_spell_effects( let mut total_resisted: u32 = 0; let mut total_absorbed: u32 = 0; let mut any_crit = false; + let mut item_grants = 0; + let mut item_refused = false; for e in effects { let points = effect_amount( e.base_points, @@ -363,6 +368,11 @@ fn run_spell_effects( dest, triggered, ); + match hit.item { + ItemGrantOutcome::None => {} + ItemGrantOutcome::Granted => item_grants += 1, + ItemGrantOutcome::Refused => item_refused = true, + } remember_hit_target(&mut hit_targets, &mut seen_hit_targets, t); // Only hits on the PRIMARY target feed the single-target damage log (crit/resist/absorb too). if t == target_guid { @@ -374,6 +384,15 @@ fn run_spell_effects( } } } + if item_refused { + if let Some(items) = inventory_before_craft { + crate::items::restore_carried_items(ctx, caster_guid, items); + } + } else { + for _ in 0..item_grants { + super::targeting::gain_recipe_skill(ctx, caster_guid, hdr.spell_id); + } + } if queues_next_swing(effects) { return; // parked on the next swing — the swing emits the GO row, not this cast } @@ -491,6 +510,7 @@ fn cast_triggered_with_origin( level, origin, None, + None, ); Ok(()) } diff --git a/module/src/spell/cast/targeting.rs b/module/src/spell/cast/targeting.rs index 9dcfc6592..1da8c6826 100644 --- a/module/src/spell/cast/targeting.rs +++ b/module/src/spell/cast/targeting.rs @@ -25,10 +25,19 @@ use crate::skilldata::game_skill_ability; // module doc — so the E_SCRIPTED arm reaches it by explicit path. use crate::script_binding::game_script; -/// What one damaging effect dealt + how the hit broke down — surfaced so the cast-GO row can carry the -/// crit flag + resisted/absorbed for the floating damage log. Non-damaging arms return `EffectHit::none()`. +#[derive(Clone, Copy, Default)] +pub(crate) enum ItemGrantOutcome { + #[default] + None, + Granted, + Refused, +} + +/// The effect's hit details and item grant outcome. The cast settles crafting before publishing +/// hit callbacks or awarding profession skill. #[derive(Clone, Copy, Default)] pub(crate) struct EffectHit { + pub item: ItemGrantOutcome, pub dealt: u32, // post-mitigation damage (what `apply_target_damage` returned) pub resisted: u32, // magic resisted off the crit-scaled hit (0 for physical / unresisted) pub absorbed: u32, // soaked by A_ABSORB shields before the health write @@ -93,6 +102,12 @@ fn recipe_skill(ctx: &ReducerContext, spell_id: u32) -> Option<(u32, u32, u32)> .next() } +pub(crate) fn gain_recipe_skill(ctx: &ReducerContext, caster_guid: u64, spell_id: u32) { + if let Some((line, green, gray)) = recipe_skill(ctx, spell_id) { + crate::skill::gain_profession_skill(ctx, caster_guid, line, green, gray); + } +} + // =========================================================================================== // Targeting + effect dispatch // =========================================================================================== @@ -1018,6 +1033,7 @@ fn apply_damage_effect( absorbed, crit: is_crit, healed: 0, + ..EffectHit::default() } } E_WEAPON_STRIKE => { @@ -1322,27 +1338,24 @@ fn apply_inventory_effect( "spell {} E_CREATE_ITEM with no item entry (p0=0) — skipped", e.spell_id ); + return EffectHit { + item: ItemGrantOutcome::Refused, + ..EffectHit::default() + }; } else if let Err(err) = crate::items::grant_item(ctx, caster_guid, entry, count) { log::info!( "spell {} E_CREATE_ITEM: could not grant {count}x item {entry} to {caster_guid}: {err}", e.spell_id ); - // CRAFT (282): the reagents were already consumed (the gate in resolve_cast_at), so a - // failed product grant (full bag) must NOT eat them — refund EVERY reagent (vanilla refunds - // an unstorable craft). A conjure/quest CreateItem has no reagents → this loop is empty. - for (item, cnt) in recipe_reagents(ctx, e.spell_id) { - let _ = crate::items::grant_item(ctx, caster_guid, item, cnt); - } - } else if let Some((line, green, gray)) = recipe_skill(ctx, e.spell_id) { - // CRAFT skill-up (282) — the profession-loop's third leg, co-located with the product grant. - // Reaching here means the craft fully succeeded → climb the recipe's OWN profession line one - // step toward its cap, with the REAL difficulty band (green floor .. gray ceiling) from - // game_skill_ability. A no-op if the line isn't learned or is at cap; a degenerate/zero band - // ⇒ always +1 (skillup_chance_bp). Non-craft E_CREATE_ITEM (conjure) has no profession - // ability → None → skipped. - crate::skill::gain_profession_skill(ctx, caster_guid, line, green, gray); + return EffectHit { + item: ItemGrantOutcome::Refused, + ..EffectHit::default() + }; + } + EffectHit { + item: ItemGrantOutcome::Granted, + ..EffectHit::default() } - EffectHit::none() } E_PICKPOCKET => { // Grant the rogue copper from the CREATURE target WITHOUT engaging (no combat, no aggro, no diff --git a/module/src/trade.rs b/module/src/trade.rs index e4195487d..9cd92b3b0 100644 --- a/module/src/trade.rs +++ b/module/src/trade.rs @@ -805,10 +805,9 @@ fn run_trade_commit( Ok(()) => { // Reserve both batches while outgoing identities still exist. The item Relay must // observe new identities for the incoming items, even when both inventories empty. - let partner_guids = - crate::items::allocate_item_guids(ctx, partner.guid, acceptor_items.len())?; - let acceptor_guids = - crate::items::allocate_item_guids(ctx, acceptor.guid, partner_items.len())?; + let mut partner_guids = + crate::items::allocate_item_guids(ctx, acceptor_items.len() + partner_items.len())?; + let acceptor_guids = partner_guids.split_off(acceptor_items.len()); // ALL outgoing rows leave first — their slots are the room the verdict counted on. let instances = ctx.db.game_item_instance(); for inst in acceptor_items.iter().chain(partner_items.iter()) { diff --git a/module/src/transfer/mod.rs b/module/src/transfer/mod.rs index 6a1fa0542..87be18dea 100644 --- a/module/src/transfer/mod.rs +++ b/module/src/transfer/mod.rs @@ -1266,7 +1266,7 @@ pub(crate) fn apply_import_blob( c.money = decoded.money; let account_id = c.account_id; // Remove stale rows for this Character even when its Character row is absent. Item import - // separately checks foreign GUID collisions; legacy packing did not enforce namespaces. + // separately checks foreign GUID collisions because legacy packing could overlap. sink.cascade_delete_character(guid); sink.insert_character(c); // AC#3: ratchet this database's guid allocator past `guid` NOW, in the same diff --git a/module/tests/account_ownership.rs b/module/tests/account_ownership.rs index 73b3907a8..8ebfdf0dd 100644 --- a/module/tests/account_ownership.rs +++ b/module/tests/account_ownership.rs @@ -40,6 +40,7 @@ fn claim_replay_recovers_a_lost_reply_without_reopening_closed_ownership() { let mut shard = Standalone::start("account-claim-replay"); shard.publish_module(); shard.assert_call("claim_operator", &[]); + shard.assert_call("install_guid_range", &["0"]); let first = token(1, 101); let deadline = claim(&shard, "101"); let receipt = shard.query_rows("SELECT * FROM game_account_claim"); @@ -115,9 +116,11 @@ fn a_new_generation_completes_partial_admission_and_fences_transfer_completion() let mut source = Standalone::start("account-transfer-source"); source.publish_module(); source.assert_call("claim_operator", &[]); + source.assert_call("install_guid_range", &["0"]); let mut destination = Standalone::start("account-transfer-destination"); destination.publish_module(); destination.assert_call("claim_operator", &[]); + destination.assert_call("install_guid_range", &["1000000000"]); let first = token(1, 201); let deadline = claim(&source, "201"); fence(&source, &first, &deadline); @@ -235,6 +238,7 @@ fn expired_account_ownership_is_reaped_while_another_gateway_keeps_its_lease_ali let mut shard = Standalone::start("account-owner-crash"); shard.publish_module(); shard.assert_call("claim_operator", &[]); + shard.assert_call("install_guid_range", &["0"]); shard.assert_call("gw_heartbeat", &[]); let first = token(1, 301); let deadline = claim(&shard, "301"); @@ -283,6 +287,7 @@ fn realm_core_party_requests_use_the_account_claim_without_a_world_shard_fence() let mut realm = Standalone::start("account-realm-party"); realm.publish_module(); realm.assert_call("claim_operator", &[]); + realm.assert_call("install_guid_range", &["0"]); claim(&realm, "401"); let first = actor(&token(1, 401)); realm.assert_call("realm_group_op", &["0", &first, "2", "0", "0"]); @@ -359,6 +364,7 @@ fn expired_fences_make_progress_in_bounded_batches() { let mut shard = Standalone::start("account-fence-batches"); shard.publish_module(); shard.assert_call("claim_operator", &[]); + shard.assert_call("install_guid_range", &["0"]); shard.assert_call("debug_spawn_player_entity", &["1"]); let mut rows: Vec<_> = (1..=65) .map(|id| format!("({id},'TEST',1,1,{id},0,false)")) diff --git a/module/tests/auction_buyout.rs b/module/tests/auction_buyout.rs index 0ad383ad7..c4a4c1366 100644 --- a/module/tests/auction_buyout.rs +++ b/module/tests/auction_buyout.rs @@ -10,6 +10,7 @@ fn real_realm_reducer_commits_exact_buyout_mail_before_the_next_transaction() { standalone.publish_module(); for (reducer, args) in [ ("claim_operator", &[][..]), + ("install_guid_range", &["0"][..]), ("debug_stage_auction_buyout_fixture", &[][..]), ( "realm_auction_decide_bid", diff --git a/module/tests/auction_expiry.rs b/module/tests/auction_expiry.rs index c77d61b3a..2cc530711 100644 --- a/module/tests/auction_expiry.rs +++ b/module/tests/auction_expiry.rs @@ -8,9 +8,9 @@ use support::Standalone; fn scheduled_bid_expiry_settles_once_and_a_callback_replay_is_a_no_op() { let mut standalone = Standalone::start("auction-expiry"); standalone.publish_module(); - for reducer in ["claim_operator", "debug_stage_auction_expiry_fixture"] { - standalone.assert_call(reducer, &[]); - } + standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); + standalone.assert_call("debug_stage_auction_expiry_fixture", &[]); standalone.wait_until_call_succeeds("debug_verify_auction_expiry_fixture", &[]); diff --git a/module/tests/binding_quests.rs b/module/tests/binding_quests.rs index cc7141218..7cdf6871f 100644 --- a/module/tests/binding_quests.rs +++ b/module/tests/binding_quests.rs @@ -197,6 +197,7 @@ fn binding_items_require_the_circle_and_complete_both_quest_variants_without_rep let mut node = Standalone::start("binding-quests"); node.publish_module(); node.assert_call("claim_operator", &[]); + node.assert_call("install_guid_range", &["0"]); node.assert_sql("UPDATE game_character SET level = 20, class = 9, race = 1 WHERE guid = 1"); node.assert_call("debug_spawn_player_entity", &[PLAYER]); node.assert_sql("UPDATE game_world_entity SET level = 20, health = 100000, max_health = 100000 WHERE guid = 1"); diff --git a/module/tests/eventai_quest_credit.rs b/module/tests/eventai_quest_credit.rs index a4df2cdf0..150f56c29 100644 --- a/module/tests/eventai_quest_credit.rs +++ b/module/tests/eventai_quest_credit.rs @@ -12,6 +12,7 @@ fn eventai_quest_credit_keeps_pinned_recipient_and_completion_contracts() { standalone.publish_module(); for (reducer, args) in [ ("claim_operator", &[][..]), + ("install_guid_range", &["0"][..]), ("debug_spawn_player_entity", &["1"][..]), ("debug_verify_eventai_quest_credit_fixture", &[][..]), ] { diff --git a/module/tests/goober_lock.rs b/module/tests/goober_lock.rs index d1ade0784..f929a987a 100644 --- a/module/tests/goober_lock.rs +++ b/module/tests/goober_lock.rs @@ -123,6 +123,7 @@ fn locked_goober_use_is_atomic_until_an_opener_unlocks_it() { let mut standalone = Standalone::start("goober-lock"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); standalone.assert_call("debug_spawn_player_entity", &[&CHARACTER_GUID.to_string()]); standalone.assert_sql(&format!( diff --git a/module/tests/item_guids.rs b/module/tests/item_guids.rs index 4a7ae681f..caeb1d060 100644 --- a/module/tests/item_guids.rs +++ b/module/tests/item_guids.rs @@ -3,12 +3,16 @@ mod support; use support::Standalone; fn fixture(name: &str) -> Standalone { + fixture_in_range(name, "0") +} + +fn fixture_in_range(name: &str, base: &str) -> Standalone { let mut shard = Standalone::start(name); shard.publish_module(); shard.assert_call("claim_operator", &[]); + shard.assert_call("install_guid_range", &[base]); shard.assert_call("debug_seed_scenario_fixtures", &[]); shard.assert_call("debug_spawn_player_entity", &["1"]); - shard.assert_call("install_guid_range", &["0"]); shard.assert_call( "create_character", &["1", "\"Other\"", "1", "1", "0", "0", "0", "0", "0", "0"], @@ -49,7 +53,8 @@ fn legacy_items_keep_their_identity_and_no_longer_control_new_grants() { .unwrap()["guid"] .parse() .unwrap(); - assert_eq!(fresh, 0x4000_8000_0000_0800); + assert_eq!(fresh >> 47, 0x8001); + assert!(fresh & ((1 << 47) - 1) < 1_000_000_000); shard.assert_call("debug_move_item", &["1", "23", "25"]); let moved = shard.query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1 AND slot = 25"); @@ -102,6 +107,27 @@ fn trade_remints_for_each_recipient_before_deleting_outgoing_items() { shard.assert_call("gw_set_trade_item", &[first, "0", "23"]); shard.assert_call("gw_set_trade_item", &[second, "0", "23"]); shard.assert_call("gw_accept_trade", &[first]); + let items_before = shard.query_rows("SELECT * FROM game_item_instance"); + let session_before = shard.query_rows("SELECT * FROM game_trade_session"); + let mark = + shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"].clone(); + shard.assert_sql("UPDATE game_guid_allocator SET high_water = 999999998 WHERE id = 0"); + assert!(!shard.call("gw_accept_trade", &[second]).status.success()); + assert_eq!( + shard.query_rows("SELECT * FROM game_item_instance"), + items_before + ); + assert_eq!( + shard.query_rows("SELECT * FROM game_trade_session"), + session_before + ); + assert_eq!( + shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + "999999998" + ); + shard.assert_sql(&format!( + "UPDATE game_guid_allocator SET high_water = {mark} WHERE id = 0" + )); shard.assert_call("gw_accept_trade", &[second]); let after = shard.query_rows("SELECT guid,owner_guid,entry FROM game_item_instance"); assert_eq!(after.len(), 2); @@ -109,7 +135,7 @@ fn trade_remints_for_each_recipient_before_deleting_outgoing_items() { assert!(!before.iter().any(|old| old["guid"] == row["guid"])); let guid: u64 = row["guid"].parse().unwrap(); let owner: u64 = row["owner_guid"].parse().unwrap(); - assert_eq!((guid & ((1 << 47) - 1)) >> 11, owner); + assert_eq!(guid >> 47, 0x8001); assert_eq!(row["entry"], if owner == 1 { "5090052" } else { "5090050" }); } shard.assert_call("debug_grant_item", &["1", "5090050", "1"]); @@ -118,19 +144,14 @@ fn trade_remints_for_each_recipient_before_deleting_outgoing_items() { } fn freeze(shard: &Standalone, id: &str) -> String { + freeze_character(shard, id, "1") +} + +fn freeze_character(shard: &Standalone, id: &str, guid: &str) -> String { + let actor = format!(r#"{{"guid":{guid},"ownership":null}}"#); shard.assert_call( "begin_transfer", - &[ - id, - r#"{"guid":1,"ownership":null}"#, - "0", - "0", - "0", - "0", - "0", - "0", - "true", - ], + &[id, &actor, "0", "0", "0", "0", "0", "0", "true"], ); let rows = shard.query_rows(&format!( "SELECT blob FROM game_transfer_out WHERE transfer_id = {id}" @@ -158,7 +179,7 @@ fn transfer_refuses_legacy_collision_then_preserves_both_formats_on_round_trip() let before = source.query_rows("SELECT guid,entry,slot FROM game_item_instance WHERE owner_guid = 1"); let blob = freeze(&source, "5090171"); - let destination = fixture("item-guid-transfer-destination"); + let destination = fixture_in_range("item-guid-transfer-destination", "1000000000"); destination.assert_sql("DELETE FROM game_world_entity WHERE guid = 1"); destination.assert_call("debug_grant_item", &["16777217", "5090050", "1"]); destination.assert_sql(&format!( @@ -195,6 +216,16 @@ fn transfer_refuses_legacy_collision_then_preserves_both_formats_on_round_trip() before ); finish(&source, &destination, "5090171"); + source.assert_call("debug_grant_item", &["16777217", "5090050", "1"]); + let next = source.query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 16777217"); + assert!(next.iter().all(|row| before + .iter() + .all(|departed| row["guid"] != departed["guid"]))); + destination.assert_call("debug_spawn_player_entity", &["1"]); + destination.assert_call("debug_grant_item", &["1", "5090050", "1"]); + let before = destination + .query_rows("SELECT guid,entry,slot FROM game_item_instance WHERE owner_guid = 1"); + assert_eq!(before.len(), 3); let back = freeze(&destination, "5090172"); source.assert_call("import_character_blob", &["5090172", &back, operator]); finish(&destination, &source, "5090172"); @@ -208,53 +239,211 @@ fn transfer_refuses_legacy_collision_then_preserves_both_formats_on_round_trip() source .query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1") .len(), - 3 + 4 ); } #[test] #[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] -fn exhaustion_preserves_partial_stacks_and_freed_identities_can_be_reused() { +fn item_and_character_creation_share_exhaustion_without_changing_partial_stacks() { let shard = fixture("item-guid-exhaustion"); - // Park rows outside carried inventory to fill the identity block independently of slot capacity. - for _ in 0..128 { - shard.assert_call("debug_grant_item", &["1", "5090050", "16"]); - shard.assert_sql("UPDATE game_item_instance SET slot = 255 WHERE owner_guid = 1"); - } - let base = 0x4000_8000_0000_0800u64; - shard.assert_sql("UPDATE game_item_template SET max_stack = 2 WHERE entry = 5090050"); - shard.assert_sql(&format!( - "UPDATE game_item_instance SET slot = 23 WHERE guid = {base}" - )); + shard.assert_call("debug_grant_item", &["1", "5090052", "1"]); + shard.assert_sql("UPDATE game_item_template SET max_stack = 2 WHERE entry = 5090052"); + shard.assert_sql("UPDATE game_guid_allocator SET high_water = 999999998 WHERE id = 0"); let before = shard.query_rows("SELECT guid,stack_count FROM game_item_instance WHERE owner_guid = 1"); - assert_eq!(before.len(), 2048); - let refusal = shard.call("debug_grant_item", &["1", "5090050", "2"]); + let refusal = shard.call("debug_grant_item", &["1", "5090052", "4"]); assert!(!refusal.status.success()); - assert!(format!( - "{}{}", - String::from_utf8_lossy(&refusal.stdout), - String::from_utf8_lossy(&refusal.stderr) - ) - .contains("ITEM_GUID_EXHAUSTED")); assert_eq!( shard.query_rows("SELECT guid,stack_count FROM game_item_instance WHERE owner_guid = 1"), before ); - let freed = base + 123; - shard.assert_sql(&format!( - "DELETE FROM game_item_instance WHERE guid = {freed}" + assert_eq!( + shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + "999999998" + ); + shard.assert_call("debug_grant_item", &["1", "5090052", "2"]); + let issued = + shard.query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1 AND slot = 24"); + assert_eq!( + issued[0]["guid"], + (0x4000_8000_0000_0000u64 | 999_999_999).to_string() + ); + shard.assert_sql("DELETE FROM game_item_instance WHERE owner_guid = 1"); + assert!(!shard + .call("debug_grant_item", &["1", "5090050", "1"]) + .status + .success()); + assert!(!shard + .call( + "create_character", + &["1", "\"Full\"", "1", "1", "0", "0", "0", "0", "0", "0"] + ) + .status + .success()); + shard.assert_call("install_guid_range", &["0"]); + assert_eq!( + shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + "999999999" + ); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn crafting_does_not_reuse_the_consumed_reagents_identity() { + let shard = fixture("item-guid-craft"); + shard.assert_call("debug_grant_item", &["1", "5090052", "1"]); + let old_guid = shard.query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1")[0] + ["guid"] + .clone(); + seed_craft(&shard); + shard.assert_call("debug_cast_at", &["1", "5090171", "1"]); + let created = + shard.query_rows("SELECT guid,entry FROM game_item_instance WHERE owner_guid = 1"); + assert_eq!(created.len(), 1); + assert_eq!(created[0]["entry"], "5090050"); + assert_ne!(created[0]["guid"], old_guid); +} + +fn seed_craft(shard: &Standalone) { + shard.assert_sql("INSERT INTO game_spell (spell_id,name,power_type,cost,cast_time_ms,gcd_ms,cooldown_ms,range_yd,duration_ms,school_mask,dispel_type,mechanic,max_stacks,aura_interrupt,attributes,spell_level,max_level,is_negative,cast_flags,stances,family_name,family_flags,proc_flags,proc_chance,proc_charges) VALUES (5090171,'Fixture craft',0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,false,0,0,0,0,0,0,0)"); + shard.assert_sql("INSERT INTO game_spell_effect (id,spell_id,effect_index,kind,base_points,die_sides,per_level,period_ms,target,radius_yd,chain_targets,trigger_spell,effect_mechanic,p0,p0_kind,p1,script_id,enters_combat) VALUES (20360684,5090171,0,7,1,0,0.0,0,0,0.0,0,0,0,5090050,8,0,0,false)"); + shard.assert_sql("INSERT INTO game_spell_reagent (id,spell_id,item_entry,count) VALUES (40721368,5090171,5090052,1)"); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn a_failed_later_craft_product_restores_reagents_without_skill_or_partial_products() { + let shard = fixture("item-guid-craft-refund"); + seed_craft(&shard); + shard.assert_sql("INSERT INTO game_spell_effect (id,spell_id,effect_index,kind,base_points,die_sides,per_level,period_ms,target,radius_yd,chain_targets,trigger_spell,effect_mechanic,p0,p0_kind,p1,script_id,enters_combat) VALUES (20360685,5090171,1,7,1,0,0.0,0,0,0.0,0,0,0,5090050,8,0,0,false)"); + shard.assert_sql("INSERT INTO game_skill_ability (id,spell_id,skill_line,race_mask,class_mask,min_skill,acquire_method,gray,green) VALUES (5090171,5090171,185,0,0,0,0,0,0)"); + shard.assert_call("debug_set_skill", &["1", "185", "1"]); + shard.assert_call("debug_grant_item", &["1", "5090052", "1"]); + shard.assert_sql("UPDATE game_guid_allocator SET high_water = 999999998 WHERE id = 0"); + let items = shard.query_rows("SELECT * FROM game_item_instance WHERE owner_guid = 1"); + let skills = shard.query_rows("SELECT * FROM game_player_skill WHERE character_guid = 1"); + let spells = shard.query_rows("SELECT * FROM game_player_spell WHERE character_guid = 1"); + shard.assert_call("debug_cast_at", &["1", "5090171", "1"]); + assert_eq!( + shard.query_rows("SELECT * FROM game_item_instance WHERE owner_guid = 1"), + items + ); + assert_eq!( + shard.query_rows("SELECT * FROM game_player_skill WHERE character_guid = 1"), + skills + ); + assert_eq!( + shard.query_rows("SELECT * FROM game_player_spell WHERE character_guid = 1"), + spells + ); + assert_eq!( + shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + "999999999" + ); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn a_high_range_character_arrives_and_receives_items_from_the_destinations_own_range() { + let source = fixture_in_range("item-guid-high-source", "1000000000"); + source.assert_call( + "create_character", + &["1", "\"Traveler\"", "1", "1", "0", "0", "0", "0", "0", "0"], + ); + let traveler = source.query_rows("SELECT guid FROM game_character WHERE name = 'Traveler'")[0] + ["guid"] + .clone(); + assert!(traveler.parse::().unwrap() > 1_000_000_000); + let before = source.query_rows(&format!( + "SELECT guid FROM game_item_instance WHERE owner_guid = {traveler}" )); - shard.assert_call("debug_grant_item", &["1", "5090050", "2"]); - let reused = shard.query_rows(&format!( - "SELECT slot,stack_count FROM game_item_instance WHERE guid = {freed}" + let blob = freeze_character(&source, "5090173", &traveler); + let destination = fixture("item-guid-low-destination"); + let mark: u64 = destination.query_rows("SELECT high_water FROM game_guid_allocator")[0] + ["high_water"] + .parse() + .unwrap(); + destination.assert_call( + "import_character_blob", + &["5090173", &blob, r#"{"guid":0,"ownership":null}"#], + ); + finish(&source, &destination, "5090173"); + assert_eq!( + destination.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + mark.to_string() + ); + destination.assert_call("debug_spawn_player_entity", &[&traveler]); + destination.assert_call("debug_grant_item", &[&traveler, "5090050", "1"]); + assert_eq!( + destination.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + (mark + 1).to_string() + ); + let after = destination.query_rows(&format!( + "SELECT guid FROM game_item_instance WHERE owner_guid = {traveler}" )); - assert_eq!(reused[0]["slot"], "24"); - assert_eq!(reused[0]["stack_count"], "1"); + assert_eq!(after.len(), before.len() + 1); + assert!(before.iter().all(|row| after.contains(row))); + let fresh = (0x4000_8000_0000_0000u64 | (mark + 1)).to_string(); + assert!(after.iter().any(|row| row["guid"] == fresh)); + let back = freeze_character(&destination, "5090174", &traveler); + source.assert_call( + "import_character_blob", + &["5090174", &back, r#"{"guid":0,"ownership":null}"#], + ); + finish(&destination, &source, "5090174"); + assert_eq!( + destination.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + (mark + 1).to_string() + ); + destination.assert_call("debug_grant_item", &["16777217", "5090050", "1"]); + assert_eq!( + destination.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + (mark + 2).to_string() + ); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn item_format_exhaustion_does_not_advance_the_range() { + let shard = fixture_in_range("item-guid-format-limit", "140737000000000"); + shard.assert_sql("UPDATE game_guid_allocator SET high_water = 140737488355327 WHERE id = 0"); + let refusal = shard.call("debug_grant_item", &["1", "5090050", "1"]); + assert!(!refusal.status.success()); + assert!(String::from_utf8_lossy(&refusal.stderr).contains("ITEM_GUID_EXHAUSTED")); + assert_eq!( + shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + "140737488355327" + ); + assert!(shard + .query_rows("SELECT guid FROM game_item_instance WHERE owner_guid = 1") + .is_empty()); +} + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn a_local_legacy_collision_refuses_without_replacing_either_owners_items() { + let shard = fixture("item-guid-local-conflict"); + shard.assert_call("debug_grant_item", &["16777217", "5090050", "1"]); + let mark: u64 = shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"] + .parse() + .unwrap(); + let collision = 0x4000_8000_0000_0000u64 | (mark + 1); + shard.assert_sql(&format!( + "UPDATE game_item_instance SET guid = {collision} WHERE owner_guid = 16777217" + )); + let before = shard.query_rows("SELECT * FROM game_item_instance"); + let refusal = shard.call("debug_grant_item", &["1", "5090050", "1"]); + assert!(!refusal.status.success()); + assert!(format!( + "{}{}", + String::from_utf8_lossy(&refusal.stdout), + String::from_utf8_lossy(&refusal.stderr) + ) + .contains("ITEM_GUID_CONFLICT")); + assert_eq!(shard.query_rows("SELECT * FROM game_item_instance"), before); assert_eq!( - shard.query_rows(&format!( - "SELECT stack_count FROM game_item_instance WHERE guid = {base}" - ))[0]["stack_count"], - "2" + shard.query_rows("SELECT high_water FROM game_guid_allocator")[0]["high_water"], + mark.to_string() ); } diff --git a/module/tests/loot_roll_promotion.rs b/module/tests/loot_roll_promotion.rs index a507ddeef..563361c19 100644 --- a/module/tests/loot_roll_promotion.rs +++ b/module/tests/loot_roll_promotion.rs @@ -88,6 +88,7 @@ fn fixture(name: &str) -> Standalone { let mut standalone = Standalone::start(name); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone } diff --git a/module/tests/package_config.rs b/module/tests/package_config.rs index b2b4e06a1..a9d218034 100644 --- a/module/tests/package_config.rs +++ b/module/tests/package_config.rs @@ -40,6 +40,7 @@ fn debug_seed_package_config_seeds_once_and_never_clobbers_a_live_value_on_resee let mut standalone = Standalone::start("package-config-seed"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call( "debug_seed_package_config", @@ -84,6 +85,7 @@ fn set_package_config_updates_an_existing_keys_value() { let mut standalone = Standalone::start("package-config-update"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call( "debug_seed_package_config", &[&arg(PACKAGE), &arg("max_bots"), &arg("10")], @@ -107,6 +109,7 @@ fn set_package_config_refuses_an_unknown_key_and_names_the_packages_known_keys() let mut standalone = Standalone::start("package-config-refusal"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call( "debug_seed_package_config", &[&arg(PACKAGE), &arg("max_bots"), &arg("10")], @@ -147,6 +150,7 @@ fn set_package_config_with_allow_new_creates_an_unknown_key() { let mut standalone = Standalone::start("package-config-allow-new"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call( "debug_seed_package_config", &[&arg(PACKAGE), &arg("max_bots"), &arg("10")], diff --git a/module/tests/package_delta_references.rs b/module/tests/package_delta_references.rs index b468b8cd7..ee6b74550 100644 --- a/module/tests/package_delta_references.rs +++ b/module/tests/package_delta_references.rs @@ -174,6 +174,7 @@ fn quest_and_loot_claims_refuse_missing_cross_table_references() { let mut standalone = Standalone::start("package-delta-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); let refused_quest = apply(&standalone, "quests", &quest_source_item(MISSING_ITEM)); @@ -215,6 +216,7 @@ fn cast_claims_refuse_missing_cross_table_references() { let mut standalone = Standalone::start("package-delta-cast-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); let refused_spell = apply( @@ -259,6 +261,7 @@ fn creature_spell_updates_cannot_hide_their_fixture_creature_target() { let mut standalone = Standalone::start("cast-fixture-target"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); assert_eq!( standalone @@ -319,6 +322,7 @@ fn a_profession_offerings_marker_spell_is_exempt_but_a_class_offerings_is_not() let mut standalone = Standalone::start("package-delta-trainer-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); // A class offering (`learn_skill_line` 0) names a real spell. @@ -376,6 +380,7 @@ fn gossip_claims_refuse_missing_cross_table_references() { let mut standalone = Standalone::start("package-delta-gossip-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); let refused_slot = apply( @@ -428,6 +433,7 @@ fn globals_claims_refuse_missing_references_and_every_insert_on_a_fixed_key_tabl let mut standalone = Standalone::start("package-delta-globals-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); let refused_zone = apply( @@ -487,6 +493,7 @@ fn spell_metadata_claims_refuse_a_missing_spell_in_the_key_and_in_a_column() { let mut standalone = Standalone::start("package-delta-spellmeta-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); let refused_chain = apply(&standalone, "spellmeta", &spell_chain_insert(PACKAGE_SPELL)); @@ -587,6 +594,7 @@ fn creature_spawn_claims_refuse_a_missing_template_and_one_row_claimed_on_two_ma let mut standalone = Standalone::start("package-delta-creature-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); let refused = apply( @@ -661,6 +669,7 @@ fn gameobject_claims_refuse_a_missing_template_and_place_an_invented_one() { let mut standalone = Standalone::start("package-delta-gameobject-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); let refused = apply( &standalone, @@ -790,6 +799,7 @@ fn creature_ai_claims_are_checked_against_quests_and_encounter_ownership() { let mut standalone = Standalone::start("package-delta-creature-ai-references"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); seed_broadcast_texts(&standalone); diff --git a/module/tests/package_runtime_scripts.rs b/module/tests/package_runtime_scripts.rs index e746debd2..5eb1ef0b2 100644 --- a/module/tests/package_runtime_scripts.rs +++ b/module/tests/package_runtime_scripts.rs @@ -95,6 +95,7 @@ fn a_packages_runtime_scripts_reconcile_onto_a_shard() { let mut standalone = Standalone::start("package-runtime-scripts"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); // --- One Package's whole set lands, disabled script included. apply( @@ -231,6 +232,7 @@ fn a_conflicting_plan_leaves_the_shard_exactly_as_it_was() { let mut standalone = Standalone::start("package-runtime-scripts-conflict"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); apply( &standalone, @@ -304,6 +306,7 @@ fn a_package_binds_its_own_event_and_never_another_packages() { let mut standalone = Standalone::start("package-runtime-scripts-package-event"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); apply( &standalone, @@ -369,6 +372,7 @@ fn a_package_script_fires_on_a_real_event_and_a_failing_one_does_not_block_the_n let mut standalone = Standalone::start("package-runtime-scripts-fire"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); standalone.assert_call("debug_spawn_player_entity", &[&PLAYER.to_string()]); diff --git a/module/tests/playerbots_personality.rs b/module/tests/playerbots_personality.rs index 984cfc838..baf0e3ae9 100644 --- a/module/tests/playerbots_personality.rs +++ b/module/tests/playerbots_personality.rs @@ -214,6 +214,7 @@ fn the_packages_own_personality_artifact_reconciles_onto_a_shard() { let mut standalone = Standalone::start("playerbots-personality-shipped"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); reconcile(&standalone, &[shipped_artifact()]); diff --git a/module/tests/quest_choice_reward.rs b/module/tests/quest_choice_reward.rs index e95708e1a..e4a34d8d5 100644 --- a/module/tests/quest_choice_reward.rs +++ b/module/tests/quest_choice_reward.rs @@ -20,6 +20,7 @@ fn durable_choice_turn_in_grants_the_selected_item_with_all_other_rewards() { standalone.publish_module(); for (reducer, args) in [ ("claim_operator", &[][..]), + ("install_guid_range", &["0"][..]), ("debug_spawn_player_entity", &["1"][..]), ("debug_stage_choice_reward_fixture", &["false"][..]), ("debug_turn_in_choice_reward_fixture", &["0"][..]), diff --git a/module/tests/random_properties.rs b/module/tests/random_properties.rs index a90bee554..175e6d0b8 100644 --- a/module/tests/random_properties.rs +++ b/module/tests/random_properties.rs @@ -10,6 +10,7 @@ fn fixture(name: &str) -> Standalone { let mut shard = Standalone::start(name); shard.publish_module(); shard.assert_call("claim_operator", &[]); + shard.assert_call("install_guid_range", &["0"]); shard.assert_call("debug_seed_scenario_fixtures", &[]); shard.assert_call("debug_spawn_player_entity", &["1"]); shard.assert_sql("DELETE FROM game_item_instance WHERE owner_guid = 1"); @@ -448,6 +449,7 @@ fn cross_shard_transfer_imports_the_saved_item_property() { let mut destination = Standalone::start("property-transfer-destination"); destination.publish_module(); destination.assert_call("claim_operator", &[]); + destination.assert_call("install_guid_range", &["1000000000"]); let blob = serde_json::to_string(out[0]["blob"].strip_prefix("0x").unwrap()).unwrap(); destination.assert_call( "import_character_blob", diff --git a/module/tests/random_property_stats.rs b/module/tests/random_property_stats.rs index 4c2d807fd..cfbf5967a 100644 --- a/module/tests/random_property_stats.rs +++ b/module/tests/random_property_stats.rs @@ -13,6 +13,7 @@ fn fixture(name: &str, class: u8) -> Standalone { let mut shard = Standalone::start(name); shard.publish_module(); shard.assert_call("claim_operator", &[]); + shard.assert_call("install_guid_range", &["0"]); shard.assert_call("debug_seed_scenario_fixtures", &[]); shard.assert_sql("DELETE FROM game_item_instance WHERE owner_guid = 1"); shard.assert_sql(&format!( diff --git a/module/tests/session_expiry.rs b/module/tests/session_expiry.rs index 5ccef13c0..31585498e 100644 --- a/module/tests/session_expiry.rs +++ b/module/tests/session_expiry.rs @@ -13,6 +13,7 @@ fn logon_renews_expired_sessions_and_the_scheduler_reaps_only_expired_rows() { let mut standalone = Standalone::start("session-expiry"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); assert!(standalone .query_rows("SELECT * FROM game_session_reaper_schedule") .is_empty()); diff --git a/module/tests/teleport_motion.rs b/module/tests/teleport_motion.rs index 2395c118b..bb34ff3fa 100644 --- a/module/tests/teleport_motion.rs +++ b/module/tests/teleport_motion.rs @@ -14,6 +14,7 @@ fn teleport_motion_rows_follow_the_live_entity_boundary() { let mut standalone = Standalone::start("teleport-motion"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_spawn_player_entity", &[&CHARACTER_GUID.to_string()]); move_to(&standalone, 100.0, 100.0, 1); diff --git a/module/tests/typed_taxi_contracts.rs b/module/tests/typed_taxi_contracts.rs index e23d2acf0..124da3f01 100644 --- a/module/tests/typed_taxi_contracts.rs +++ b/module/tests/typed_taxi_contracts.rs @@ -17,6 +17,7 @@ fn gateway_taxi_gates_keep_refusals_typed_and_invariants_fatal() { standalone.publish_module_bytes(wasm); assert_loot_boundary_failure(&standalone, ACTOR, "loot:boundary_operator_rejected"); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_spawn_player_entity", &[ACTOR]); standalone.assert_sql( "INSERT INTO game_active_taxi_flight \ diff --git a/module/tests/weather.rs b/module/tests/weather.rs index cf445a884..bd2483284 100644 --- a/module/tests/weather.rs +++ b/module/tests/weather.rs @@ -37,6 +37,7 @@ fn forced_weather_is_per_zone_and_a_refusal_leaves_every_row_unchanged() { let mut standalone = Standalone::start("weather-forced"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); // The seeded climate covers Elwynn Forest and Westfall, and no zone has weather until something // gives it some. @@ -126,6 +127,7 @@ fn the_weather_roll_and_its_climate_survive_a_republish() { let mut standalone = Standalone::start("weather-republish"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("gw_force_zone_weather", &[ELWYNN, RAIN, "0.75"]); assert_eq!( diff --git a/module/tests/xp_cap.rs b/module/tests/xp_cap.rs index 4878003ce..6bd8f8e9a 100644 --- a/module/tests/xp_cap.rs +++ b/module/tests/xp_cap.rs @@ -11,6 +11,7 @@ fn the_level_cap_discards_excess_xp_and_preserves_rested_xp_on_later_kills() { let mut standalone = Standalone::start("xp-cap"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call("debug_seed_scenario_fixtures", &[]); standalone.assert_call("debug_spawn_player_entity", &["1"]); standalone.assert_call("debug_set_level", &["1", "59"]); diff --git a/module/tests/zone_transition.rs b/module/tests/zone_transition.rs index 63c7ee476..53a63d20d 100644 --- a/module/tests/zone_transition.rs +++ b/module/tests/zone_transition.rs @@ -37,6 +37,7 @@ fn accepted_movement_across_a_zone_boundary_updates_the_live_and_durable_zone_on let mut standalone = Standalone::start("zone-transition"); standalone.publish_module(); standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); import_areas(&standalone); import_terrain(&standalone); From e4c5d95b8c185a73f356ec364b86e706d9df16cd Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 06:02:32 +0000 Subject: [PATCH 3/7] docs(items): state range capacity and rollout prerequisites --- docs/design/item-guids.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/design/item-guids.md b/docs/design/item-guids.md index 9c92aa38d..07accf7b1 100644 --- a/docs/design/item-guids.md +++ b/docs/design/item-guids.md @@ -4,7 +4,8 @@ Characters and items consume the same durable high-water mark in a Shard's GUID Realm-core assigns disjoint billion-value ranges. `install_guid_range` refuses reassignment; deleting a Character, deleting an item, Transfer and reinstalling the same range never rewind the mark. A batch must fit below the range's exclusive end before the allocator advances. -The range now budgets Character creation and item creation together. +The range now budgets Character creation and item creation together. Its base is the allocator +floor, leaving 999,999,999 issued identities per range. New items retain `HIGHGUID_ITEM = 0x4000` in bits 48 through 63. Bit 47 marks this allocation format. Bits 0 through 46 hold the issued value, so 140,737,488,355,327 is the largest usable @@ -47,6 +48,10 @@ with bit 47 set, missing or overlapping GUID Ranges, and allocator marks at or b range end. Resolve those cases under separate human approval. Local collision checks do not replace this realm-wide audit. An already escaped Character GUID also needs explicit review. +Without an installed range, Character cleanup no longer raises the allocator mark. Resolve a +missing range before this rollout; do not delete legacy Characters first and then seed a mark +from surviving rows. The update never invents a range assignment or resets a mark. + Pause World Sessions and all item-granting or ownership-changing activity during the update. Record outstanding Transfer Escrow. Verify each Shard's permanent range assignment, publish the reviewed Module to every Shard, and confirm that all Shards run the same build before resuming From 6e589a341d418ea95f40c1a3e7fb579da25a49d6 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 06:11:00 +0000 Subject: [PATCH 4/7] test(items): install the Loot Tag fixture GUID Range --- module/tests/loot_tag.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/module/tests/loot_tag.rs b/module/tests/loot_tag.rs index eec1bfa80..16a553178 100644 --- a/module/tests/loot_tag.rs +++ b/module/tests/loot_tag.rs @@ -8,5 +8,7 @@ use support::Standalone; fn loot_tag_owns_threat_lifetime_rewards_and_corpse_eligibility() { let mut standalone = Standalone::start("loot-tag"); standalone.publish_module_anonymous(); + standalone.assert_call("claim_operator", &[]); + standalone.assert_call("install_guid_range", &["0"]); standalone.assert_call_anonymous("debug_verify_loot_tag_fixture", &[]); } From f9b281f36b7b86419329ff504cd9c79b46f0dbe0 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 06:33:19 +0000 Subject: [PATCH 5/7] test(items): install ranges before deleting fixture Characters --- .../stdb/subscriptions_character_gone_durable_tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs b/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs index 7c3577667..11f36856c 100644 --- a/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs +++ b/gateway/src/stdb/subscriptions_character_gone_durable_tests.rs @@ -133,6 +133,10 @@ fn another_gateway_waits_for_the_transfer_then_cleans_the_deleted_character() { let output = cli.call(standalone.server(), database, "claim_operator", &[]); standalone.assert_output_success(&output, &format!("claim {database} operator")); } + for (database, base) in [(INSTANCES, "0"), (OTHER, "1000000000")] { + let output = cli.call(standalone.server(), database, "install_guid_range", &[base]); + standalone.assert_output_success(&output, &format!("install range on {database}")); + } for database in [INSTANCES, OTHER] { let output = cli.call( standalone.server(), @@ -142,10 +146,6 @@ fn another_gateway_waits_for_the_transfer_then_cleans_the_deleted_character() { ); standalone.assert_output_success(&output, &format!("clear Character 1 on {database}")); } - for (database, base) in [(INSTANCES, "0"), (OTHER, "1000000000")] { - let output = cli.call(standalone.server(), database, "install_guid_range", &[base]); - standalone.assert_output_success(&output, &format!("install range on {database}")); - } for (database, name) in [(INSTANCES, "PartyTwo"), (OTHER, "PartyOther")] { let output = cli.call( standalone.server(), From 65dca64510de42e944ce67031dc80528f23da103 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 06:40:25 +0000 Subject: [PATCH 6/7] fix(items): preserve legacy GUID floors before range setup --- docs/design/item-guids.md | 12 +++++++----- module/src/auth.rs | 28 ++++++++-------------------- module/tests/item_guids.rs | 30 ++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/docs/design/item-guids.md b/docs/design/item-guids.md index 07accf7b1..4f504906a 100644 --- a/docs/design/item-guids.md +++ b/docs/design/item-guids.md @@ -28,8 +28,8 @@ Trading creates new recipient items. Mail, Auctions and Buyback store item snaps create new items when the recipient takes them. Preserved identities now rely on disjoint GUID Ranges and a durable issuer mark, rather than a packed Character namespace. -Transfer does not move or replace a Shard's allocator. An arriving Character raises the local -mark only when its GUID belongs to the destination's own range. Incoming items need no floor +Transfer does not move or replace a Shard's allocator. With a range installed, an arriving +Character raises the local mark only when its GUID belongs to the destination's own range. Incoming items need no floor update because the issuing Shard retains their high-water mark after they leave. ## Compatibility and rollout @@ -48,9 +48,11 @@ with bit 47 set, missing or overlapping GUID Ranges, and allocator marks at or b range end. Resolve those cases under separate human approval. Local collision checks do not replace this realm-wide audit. An already escaped Character GUID also needs explicit review. -Without an installed range, Character cleanup no longer raises the allocator mark. Resolve a -missing range before this rollout; do not delete legacy Characters first and then seed a mark -from surviving rows. The update never invents a range assignment or resets a mark. +Without an installed range, Character cleanup retains the legacy allocator floor. Gateway +startup can continue after a failed range claim, so deleting a Character in that state must not +allow its GUID to be reused when provisioning recovers. Item creation still returns +`NO_GUID_RANGE`. Resolve missing ranges before deployment; the update never invents an assignment +or resets a mark. Pause World Sessions and all item-granting or ownership-changing activity during the update. Record outstanding Transfer Escrow. Verify each Shard's permanent range assignment, publish the diff --git a/module/src/auth.rs b/module/src/auth.rs index 5ec4b7092..a92a785e6 100644 --- a/module/src/auth.rs +++ b/module/src/auth.rs @@ -511,7 +511,7 @@ mod session_expiry_tests { // =========================================================================================== /// Shared durable high-water mark for Character and item creation. Deletion and Transfer never -/// lower it. Transfer advances it for an arriving Character only inside this Shard's own range. +/// lower it. With a range installed, only local Character GUIDs advance the Transfer floor. /// A scan of surviving rows cannot replace it, because issued identities may now live elsewhere. #[table(accessor = game_guid_allocator)] pub struct GuidAllocator { @@ -648,8 +648,9 @@ fn guid_batch_end( Ok(last) } -/// Preserve a local Character's issued GUID before import or deletion. Foreign Characters must -/// never advance this Shard's mark, including the cleanup that precedes a Transfer import. +/// Preserve a Character's issued GUID before import or deletion. Once a range is installed, +/// foreign Characters cannot advance its mark, including cleanup before a Transfer import. +/// Before installation, retain the legacy floor so a provisioning failure cannot erase it. pub(crate) fn bump_guid_high_water(ctx: &ReducerContext, guid: u64) { let range = ctx .db @@ -657,7 +658,7 @@ pub(crate) fn bump_guid_high_water(ctx: &ReducerContext, guid: u64) { .id() .find(0) .map(|row| (row.base, row.size)); - if !in_guid_range(range, guid) { + if range.is_some() && !in_guid_range(range, guid) { return; } let existing = read_high_water(ctx); @@ -701,21 +702,8 @@ pub(crate) fn require_guid_range(ctx: &ReducerContext) -> Result<(), String> { may_mint(range, mark.unwrap_or(0)) } -/// Is `guid` inside `range`? Pure, same reason as [`may_mint`] — testable without a -/// `ReducerContext`. `transfer::apply_import_blob` calls this to decide whether an -/// ARRIVING character's guid may ratchet THIS database's `game_guid_allocator` — ranges are -/// disjoint by construction, so a guid outside this -/// database's own range belongs to another shard and can never collide with anything this shard -/// mints; ratcheting past it anyway is pure self-harm (it walks this shard's own mark toward, or -/// past, its own range end for a guid it will never be asked to re-mint — the live -/// incident). -/// -/// `None` (no range installed yet) is conservatively `false`, i.e. "not inside" — never a bump. -/// Same reasoning as `may_mint`'s `NO_GUID_RANGE`: a database with no range cannot mint locally -/// either, so there is nothing local yet for a foreign arrival to threaten, and treating an -/// unranged guid as "inside" would let an import inflate the mark before a range even exists to -/// check it against (poisoning the eventual `install_guid_range` — see its own "already minted -/// up to N" guard). +/// True only inside an installed range. Transfer uses this Gate directly; legacy deletion +/// retains its old high-water floor until a range is installed. pub(crate) fn in_guid_range(range: Option<(u64, u64)>, guid: u64) -> bool { let Some((base, size)) = range else { return false; @@ -1023,7 +1011,7 @@ mod guid_allocator_tests { .id() .find(0) .map(|row| (row.base, row.size)); - if !in_guid_range(range, guid) { + if range.is_some() && !in_guid_range(range, guid) { return; } let existing = read_high_water(ctx); diff --git a/module/tests/item_guids.rs b/module/tests/item_guids.rs index caeb1d060..9181a9034 100644 --- a/module/tests/item_guids.rs +++ b/module/tests/item_guids.rs @@ -447,3 +447,33 @@ fn a_local_legacy_collision_refuses_without_replacing_either_owners_items() { mark.to_string() ); } + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn legacy_deletion_preserves_its_floor_before_range_installation() { + let mut shard = Standalone::start("item-guid-legacy-floor"); + shard.publish_module(); + shard.assert_call("claim_operator", &[]); + shard.assert_call("debug_delete_character", &["1"]); + let marks = shard.query_rows("SELECT high_water FROM game_guid_allocator"); + assert_eq!(marks.len(), 1); + assert_eq!(marks[0]["high_water"], "1"); + shard.assert_call("install_guid_range", &["0"]); + shard.assert_call( + "create_character", + &[ + "1", + "\"AfterDelete\"", + "1", + "1", + "0", + "0", + "0", + "0", + "0", + "0", + ], + ); + let created = shard.query_rows("SELECT guid FROM game_character WHERE name = 'AfterDelete'"); + assert_eq!(created[0]["guid"], "2"); +} From c0ddfd010db2185cd75a5ebc35f3cfbaee3e0b3e Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 06:45:15 +0000 Subject: [PATCH 7/7] docs(items): clarify legacy GUID floor preservation --- module/src/auth.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/module/src/auth.rs b/module/src/auth.rs index a92a785e6..edabc3397 100644 --- a/module/src/auth.rs +++ b/module/src/auth.rs @@ -837,8 +837,7 @@ mod guid_allocator_tests { #[test] fn in_guid_range_only_admits_guids_inside_the_installed_range() { use super::in_guid_range; - // No range installed: nothing is "inside" it — a database that cannot mint locally - // either has nothing local yet for a foreign arrival to threaten. + // No installed range proves membership. Legacy deletion preserves its floor separately. assert!(!in_guid_range(None, 0)); assert!(!in_guid_range(None, 4242)); // Inside a real range, base included, end excluded — same boundary shape as `may_mint`. @@ -1028,10 +1027,8 @@ mod guid_allocator_tests { ); assert_eq!( got, want, - "bump_guid_high_water's body no longer matches the pinned shape — it must seed from \ - the durable mark first, floor it at `guid` via the tested `ratchet_high_water`, and \ - persist the result for local GUIDs (skipping the write on the already-seeded, already-ahead \ - no-op). Got:\n{got}" + "bump_guid_high_water must preserve the legacy floor before range installation, \ + then floor only local GUIDs. It must persist the new mark unless already ahead. Got:\n{got}" ); } }