diff --git a/crates/lyracore-package-delta/src/delta.rs b/crates/lyracore-package-delta/src/delta.rs index b75bc1189..817312281 100644 --- a/crates/lyracore-package-delta/src/delta.rs +++ b/crates/lyracore-package-delta/src/delta.rs @@ -570,19 +570,14 @@ impl PrimaryKey { /// Names a caster creature's single-spell cast row. /// - /// No fixture or band check: the table is update-only (every insert on it is refused - /// regardless of identifier — see [`DeltaError::InsertNotSupported`]), and its key names a - /// creature template, which is out of this crate's scope to police. + /// The table is update-only, so it has no Package identifier band. Its key still names a + /// creature template and takes the creature fixture policy. /// /// # Errors - /// [`DeltaError::MalformedKey`] for creature entry 0. + /// [`DeltaError::MalformedKey`] for creature entry 0 or an entry the spawn guid's field cannot + /// hold, and [`DeltaError::CreatureIdFixtureReserved`] for a seeded fixture creature. pub fn creature_cast(creature_entry: u32) -> Result { - if creature_entry == 0 { - return Err(DeltaError::MalformedKey { - table: Table::CreatureCast, - detail: "`creature_entry` 0 is not a creature".to_owned(), - }); - } + check_claimable_creature_entry(Table::CreatureCast, "creature_entry", creature_entry)?; Ok(Self::CreatureCast { creature_entry }) } @@ -608,8 +603,8 @@ impl PrimaryKey { /// Names the greeting a creature template shows. /// - /// No fixture or band check: the table is update-only, and its key names a creature template, - /// which is out of this crate's scope to police — the `game_creature_cast` shape. + /// No fixture or band check: the table is update-only, and its key names a creature template. + /// Unlike `game_creature_cast`, this table does not yet apply the creature fixture policy. /// /// # Errors /// [`DeltaError::MalformedKey`] for creature entry 0. @@ -805,7 +800,7 @@ impl PrimaryKey { /// [`DeltaError::MalformedKey`] for entry 0 or an entry the spawn guid's 24-bit field cannot /// hold, and [`DeltaError::CreatureIdFixtureReserved`] for a seeded fixture creature. pub fn creature_template(entry: u32) -> Result { - check_claimable_creature_entry(entry)?; + check_claimable_creature_entry(Table::CreatureTemplate, "entry", entry)?; Ok(Self::CreatureTemplate { entry }) } @@ -1467,10 +1462,14 @@ fn check_race_and_class(table: Table, race: u8, class: u8) -> Result<(), DeltaEr Ok(()) } -/// A creature TEMPLATE entry is refused the same way under every operation when it is 0, wider -/// than the spawn guid's field, or fixture-owned. -fn check_claimable_creature_entry(entry: u32) -> Result<(), DeltaError> { - check_guid_component(Table::CreatureTemplate, "entry", entry)?; +/// A creature template entry is refused wherever a claim names one when it is 0, wider than the +/// spawn guid's field, or fixture-owned. +fn check_claimable_creature_entry( + table: Table, + member: &str, + entry: u32, +) -> Result<(), DeltaError> { + check_guid_component(table, member, entry)?; if is_fixture_reserved_creature_id(entry) { return Err(DeltaError::CreatureIdFixtureReserved { id: entry }); } @@ -1623,6 +1622,12 @@ impl Claim { } } + if let (Table::CreatureSpell, Some(FieldValue::U32(creature_entry))) = + (table, fields.get("creature_entry")) + { + check_claimable_creature_entry(table, "creature_entry", *creature_entry)?; + } + match operation { Operation::Insert => { let missing: Vec = table diff --git a/crates/lyracore-package-delta/tests/cast_identifiers.rs b/crates/lyracore-package-delta/tests/cast_identifiers.rs index 4ac5730d9..172f57c13 100644 --- a/crates/lyracore-package-delta/tests/cast_identifiers.rs +++ b/crates/lyracore-package-delta/tests/cast_identifiers.rs @@ -1,7 +1,7 @@ -//! The casts family's identifier policy: `game_creature_spell` follows the loot shape (band -//! checked against the row's own surrogate `id`). `game_creature_cast` follows no band at all — -//! it is update-only, refused unconditionally on `insert` — the worked example of -//! [`DeltaError::InsertNotSupported`]. +//! The casts family's identifier policy: `game_creature_spell` follows the loot shape, with its +//! band checked against the row's own surrogate `id`. `game_creature_cast` has no Package band +//! because it is update-only and every `insert` is refused. Both tables refuse the Module's +//! fixture creature cluster at their own creature-entry boundary. mod common; @@ -11,7 +11,8 @@ use common::{ }; use lyracore_package_delta::{ is_fixture_reserved_cast_id, is_package_cast_id, DeltaError, PackageDelta, PrimaryKey, Table, - PACKAGE_CAST_ID_CEIL, PACKAGE_CAST_ID_FLOOR, + FIXTURE_CREATURE_ID_CEIL, FIXTURE_CREATURE_ID_FLOOR, PACKAGE_CAST_ID_CEIL, + PACKAGE_CAST_ID_FLOOR, }; fn artifact(package: &str, claims: &str) -> String { @@ -63,9 +64,12 @@ fn an_insert_outside_the_package_range_is_refused() { #[test] fn an_update_may_name_a_real_imported_creature_spell_row() { + let fields = format!( + r#"{{"creature_entry":{{"type":"u32","value":{REAL_CREATURE_CAST}}},"priority":{{"type":"u8","value":20}}}}"# + ); let json = artifact( "example.pkg", - &creature_spell_claim(REAL_CREATURE_SPELL, "update", A_PRIORITY), + &creature_spell_claim(REAL_CREATURE_SPELL, "update", &fields), ); let delta = PackageDelta::parse(&json).expect("a real creature spell row is updatable"); @@ -73,6 +77,22 @@ fn an_update_may_name_a_real_imported_creature_spell_row() { assert_eq!(delta.claims()[0].key().row_id(), REAL_CREATURE_SPELL); } +#[test] +fn a_creature_spell_cannot_target_a_fixture_cluster_creature() { + for id in [FIXTURE_CREATURE_ID_FLOOR, 51_001, FIXTURE_CREATURE_ID_CEIL] { + let fields = format!(r#"{{"creature_entry":{{"type":"u32","value":{id}}}}}"#); + let json = artifact( + "example.pkg", + &creature_spell_claim(REAL_CREATURE_SPELL, "update", &fields), + ); + + assert_eq!( + PackageDelta::parse(&json).expect_err("the claim must be refused"), + DeltaError::CreatureIdFixtureReserved { id } + ); + } +} + #[test] fn no_operation_may_claim_the_project_wide_fixture_reserved_band_for_creature_spell() { for id in [5_090_000u64, 5_090_050, 5_099_999] { @@ -188,6 +208,21 @@ fn an_update_on_a_real_creature_entry_is_accepted() { ); } +#[test] +fn a_creature_cast_cannot_target_a_fixture_cluster_creature() { + for id in [FIXTURE_CREATURE_ID_FLOOR, 51_001, FIXTURE_CREATURE_ID_CEIL] { + let json = artifact( + "example.pkg", + &creature_cast_claim(id, "update", A_SPELL_ID), + ); + + assert_eq!( + PackageDelta::parse(&json).expect_err("the claim must be refused"), + DeltaError::CreatureIdFixtureReserved { id } + ); + } +} + #[test] fn creature_cast_entry_zero_is_not_a_creature() { let json = artifact("example.pkg", &creature_cast_claim(0, "update", A_SPELL_ID)); diff --git a/module/src/package_import/casts.rs b/module/src/package_import/casts.rs index d8763a8fe..206e1effd 100644 --- a/module/src/package_import/casts.rs +++ b/module/src/package_import/casts.rs @@ -6,10 +6,11 @@ //! true of casts. //! //! `game_creature_cast` is update-only ([`lyracore_package_delta::DeltaError::InsertNotSupported`]): -//! its key names a creature template, out of this family's scope to invent. It carries no Package -//! band, so [`update_target`] and `write_row` never treat it as `Uninvented`, and -//! [`clear_package_range`] never touches it. `game_creature_spell` follows the loot shape instead: -//! its own surrogate `id` is the band, checked by `is_package_cast_id`. +//! its key names a creature template and the parser refuses the fixture cluster. It carries no +//! Package band, so [`update_target`] and `write_row` never treat it as `Uninvented`, and +//! [`clear_package_range`] never touches it. `game_creature_spell` follows the loot shape instead. +//! Its own surrogate `id` is the band, checked by `is_package_cast_id`, while this family's +//! reference Gate checks both the existing and final creature target. //! //! The Claim schema and the setters below are one contract; the tests at the bottom fail if a //! claimable column has no setter. @@ -17,7 +18,8 @@ use spacetimedb::{ReducerContext, Table}; use lyracore_package_delta::{ - is_package_cast_id, FieldValue, Operation, PrimaryKey, Table as ClaimTable, TracedRow, + is_fixture_reserved_creature_id, is_package_cast_id, DeltaError, FieldValue, Operation, + PrimaryKey, Table as ClaimTable, TracedRow, }; use crate::creatures::{game_creature_cast, game_creature_spell, CreatureCast, CreatureSpell}; @@ -25,28 +27,49 @@ use crate::{game_creature_template, game_spell}; use super::{as_u32, as_u8, check_insert_is_whole, UpdateTarget}; -/// Refuses a cast claim whose final row would point at data this Shard does not hold. +/// Refuses a cast claim whose final row would point at data this Shard does not hold or whose +/// existing or final target belongs to the fixture creature cluster. pub(super) fn check_references(ctx: &ReducerContext, rows: &[TracedRow]) -> Result<(), String> { for row in rows { + if row.table() == ClaimTable::CreatureSpell { + check_creature_spell_target(ctx, row)?; + } + let spell_id = final_u32(ctx, row, "spell_id")?; if spell_id == 0 || ctx.db.game_spell().spell_id().find(spell_id).is_none() { return Err(missing_reference(row, "spell_id", spell_id)); } + } + Ok(()) +} - if row.table() == ClaimTable::CreatureSpell { - let creature_entry = final_u32(ctx, row, "creature_entry")?; - if creature_entry == 0 - || ctx - .db - .game_creature_template() - .entry() - .find(creature_entry) - .is_none() - { - return Err(missing_reference(row, "creature_entry", creature_entry)); - } +/// Refuses both a final fixture target and an existing fixture target a claim tries to hide by +/// omitting or replacing `creature_entry`. +fn check_creature_spell_target(ctx: &ReducerContext, row: &TracedRow) -> Result<(), String> { + let current = ctx + .db + .game_creature_spell() + .id() + .find(row.row_id()) + .map(|spell| spell.creature_entry); + let final_entry = final_u32(ctx, row, "creature_entry")?; + + for entry in current.into_iter().chain([final_entry]) { + if is_fixture_reserved_creature_id(entry) { + return Err(fixture_creature_reference(row, entry)); } } + + if final_entry == 0 + || ctx + .db + .game_creature_template() + .entry() + .find(final_entry) + .is_none() + { + return Err(missing_reference(row, "creature_entry", final_entry)); + } Ok(()) } @@ -99,6 +122,15 @@ fn missing_reference(row: &TracedRow, field: &str, value: u32) -> String { ) } +fn fixture_creature_reference(row: &TracedRow, entry: u32) -> String { + format!( + "`{}` row {} references {}", + row.table(), + row.key(), + DeltaError::CreatureIdFixtureReserved { id: entry } + ) +} + /// What this shard holds for the row an `update` claim names. pub(super) fn update_target(ctx: &ReducerContext, row: &TracedRow) -> UpdateTarget { // Only `game_creature_spell` has a Package band to reconcile against — `game_creature_cast` diff --git a/module/tests/package_delta_references.rs b/module/tests/package_delta_references.rs index 749ce5552..8b907b89a 100644 --- a/module/tests/package_delta_references.rs +++ b/module/tests/package_delta_references.rs @@ -8,6 +8,7 @@ const HASH: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789ab const FIXTURE_QUEST: u32 = 50_900; const FIXTURE_CREATURE: u32 = 51_000; // Test Wolf. const FIXTURE_TRAINER: u32 = 51_001; // Profession Trainer. +const IMPORTED_CREATURE: u32 = 620; // Chicken, a real catalogue creature seeded at init. const REAL_ITEM: u32 = 25; const MISSING_ITEM: u32 = 4_000_000; const PACKAGE_LOOT: u64 = 9_000_001; @@ -15,6 +16,7 @@ const REAL_SPELL: u32 = 50_310; // Test Riding Horse, seeded by the land-mount f const MISSING_SPELL: u32 = 4_000_000; const MISSING_CREATURE: u32 = 4_000_000; const PACKAGE_CREATURE_SPELL: u64 = 10_000_001; +const FIXTURE_TARGETED_CREATURE_SPELL: u64 = 4_000_001; const PACKAGE_TRAINER_SPELL: u64 = 11_000_001; const PACKAGE_NPC_TEXT: u64 = 12_000_001; const PACKAGE_NPC_TEXT_SLOT: u64 = 12_000_002; @@ -64,6 +66,15 @@ fn creature_spell_insert(creature_entry: u32, spell_id: u32) -> String { ) } +fn creature_spell_update(id: u64, fields: &str) -> String { + artifact( + "example.cast", + &format!( + r#"{{"table":"game_creature_spell","key":{{"id":{id}}},"operation":"update","fields":{fields}}}"# + ), + ) +} + fn trainer_spell_insert(spell_id: u32, learn_skill_line: u32) -> String { artifact( "example.trainer", @@ -209,7 +220,7 @@ fn cast_claims_refuse_missing_cross_table_references() { let refused_spell = apply( &standalone, "casts", - &creature_spell_insert(FIXTURE_CREATURE, MISSING_SPELL), + &creature_spell_insert(IMPORTED_CREATURE, MISSING_SPELL), ); assert!(!refused_spell.status.success()); assert!( @@ -233,7 +244,7 @@ fn cast_claims_refuse_missing_cross_table_references() { let accepted = apply( &standalone, "casts", - &creature_spell_insert(FIXTURE_CREATURE, REAL_SPELL), + &creature_spell_insert(IMPORTED_CREATURE, REAL_SPELL), ); assert!(accepted.status.success(), "{}", refusal_text(&accepted)); let spell = standalone.query_rows(&format!( @@ -242,6 +253,62 @@ fn cast_claims_refuse_missing_cross_table_references() { assert_eq!(spell[0]["spell_id"], REAL_SPELL.to_string()); } +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn creature_spell_updates_cannot_hide_their_fixture_creature_target() { + let standalone = Standalone::start("cast-fixture-target"); + standalone.publish_module(); + standalone.assert_call("claim_operator", &[]); + standalone.assert_call("debug_seed_scenario_fixtures", &[]); + assert_eq!( + standalone + .query_rows(&format!( + "SELECT * FROM game_creature_template WHERE entry = {IMPORTED_CREATURE}" + )) + .len(), + 1, + "the imported target must exist" + ); + standalone.assert_sql(&format!( + "INSERT INTO game_creature_spell (id, creature_entry, spell_id, priority, condition, condition_value) VALUES ({FIXTURE_TARGETED_CREATURE_SPELL}, {FIXTURE_CREATURE}, {REAL_SPELL}, 10, 0, 0)" + )); + + let partial = apply( + &standalone, + "casts", + &creature_spell_update( + FIXTURE_TARGETED_CREATURE_SPELL, + r#"{"priority":{"type":"u8","value":20}}"#, + ), + ); + let retargeted = apply( + &standalone, + "casts", + &creature_spell_update( + FIXTURE_TARGETED_CREATURE_SPELL, + &format!(r#"{{"creature_entry":{{"type":"u32","value":{IMPORTED_CREATURE}}}}}"#), + ), + ); + + assert!( + !partial.status.success() && !retargeted.status.success(), + "partial update success: {}; retarget-away success: {}", + partial.status.success(), + retargeted.status.success() + ); + for refusal in [&partial, &retargeted] { + let text = refusal_text(refusal); + assert!(text.contains("fixture-reserved"), "{text}"); + assert!(text.contains(&FIXTURE_CREATURE.to_string()), "{text}"); + } + + let spell = standalone.query_rows(&format!( + "SELECT * FROM game_creature_spell WHERE id = {FIXTURE_TARGETED_CREATURE_SPELL}" + )); + assert_eq!(spell[0]["creature_entry"], FIXTURE_CREATURE.to_string()); + assert_eq!(spell[0]["priority"], "10"); +} + /// `game_trainer_spell.spell_id` is checked against `game_spell` only when the row's final /// `learn_skill_line` is 0. A profession offering's `spell_id` is a synthetic marker /// (`module/src/skill.rs`'s `LEARN_COOKING_SPELL_ID` and siblings) that never resolves to a