From 1a44f73d0b3a4dbd26927290c54892e3e997cda1 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sat, 26 Sep 2026 23:52:09 +0800 Subject: [PATCH 1/4] fix(element-count): align costs with Workshop evidence Fixes #305 --- .../workshop-rs/src/analysis/element_count.rs | 132 ++++--- crates/workshop-rs/tests/element_count.rs | 346 ++++++++++++++++-- docs/element-count.md | 66 ++-- 3 files changed, 451 insertions(+), 93 deletions(-) diff --git a/crates/workshop-rs/src/analysis/element_count.rs b/crates/workshop-rs/src/analysis/element_count.rs index 0e8ddd9..a01ecfd 100644 --- a/crates/workshop-rs/src/analysis/element_count.rs +++ b/crates/workshop-rs/src/analysis/element_count.rs @@ -1,13 +1,3 @@ -//! Canonical Workshop element-count analysis. -//! -//! The calculator operates on the canonical public program, not source-language syntax or -//! emitted text. Its rules are the documented Workshop.codes model: rules, -//! actions, conditions, and ordinary values cost one element; arrays and -//! evaluate-once values cost two; localized strings cost two; direct action or -//! condition arguments are reduced by one; and every pair of hero literals in -//! those arguments adds one. Custom game settings and rule parameters cost -//! zero. - use std::collections::HashMap; use std::fmt; @@ -212,19 +202,6 @@ impl Counter<'_> { let node_id = self.next_node_id(); let mut children = Vec::with_capacity(rule.conditions.len() + rule.actions.len()); for condition in &rule.conditions { - if condition.disabled { - return Err(ElementCountError::Unsupported { - kind: ElementNodeKind::Condition, - name: "disabled condition".to_string(), - span: self - .program - .values - .get(condition.value) - .and_then(|v| v.span), - reason: "the element cost of a disabled condition is not established" - .to_string(), - }); - } children.push(self.condition(condition.value)?.node); } for action in &rule.actions { @@ -305,6 +282,7 @@ impl Counter<'_> { let span = action.span(); let mut children = Vec::new(); let mut heroes = 0; + let mut adjustment = 0; let name; match action { Action::SetGlobalVariable { value, .. } @@ -332,26 +310,36 @@ impl Counter<'_> { .. } => { name = "if"; - for branch in branches { + for (index, branch) in branches.iter().enumerate() { self.push_action_value(&mut children, &mut heroes, branch.condition)?; for nested in &branch.body { children.push(self.action(*nested)?.node); } + if index > 0 { + adjustment += 1; + } } if let Some(body) = else_body { + adjustment += 1; for nested in body { children.push(self.action(*nested)?.node); } } + if !branches.is_empty() { + adjustment += 1; + } } Action::While { - condition, body, .. + condition: condition_id, + body, + .. } => { name = "while"; - self.push_action_value(&mut children, &mut heroes, *condition)?; + self.push_action_value(&mut children, &mut heroes, *condition_id)?; for nested in body { children.push(self.action(*nested)?.node); } + adjustment += 1; } Action::ForGlobalVariable { start, @@ -367,6 +355,7 @@ impl Counter<'_> { for nested in body { children.push(self.action(*nested)?.node); } + adjustment += 1; } Action::ForPlayerVariable { player, @@ -383,15 +372,9 @@ impl Counter<'_> { for nested in body { children.push(self.action(*nested)?.node); } + adjustment += 1; } - Action::Disabled { .. } => { - return Err(ElementCountError::Unsupported { - kind: ElementNodeKind::Action, - name: "disabled action".to_string(), - span, - reason: "the element cost of a disabled action is not established".to_string(), - }); - } + Action::Disabled { action, .. } => return self.action(*action), Action::Call { name: action_name, args, @@ -406,8 +389,13 @@ impl Counter<'_> { }); } name = action_name.as_str(); - for argument in args { - self.push_action_value(&mut children, &mut heroes, *argument)?; + for (index, argument) in args.iter().enumerate() { + if parameter_is_variable_reference(self.catalog, action_name, index) { + continue; + } + let counted = self.value(*argument, true)?; + heroes += counted.heroes; + children.push(counted.node); } } } @@ -417,7 +405,7 @@ impl Counter<'_> { name, span, 1, - pair_surcharge(heroes), + adjustment + pair_surcharge(heroes), children, heroes, )) @@ -451,7 +439,7 @@ impl Counter<'_> { }; let span = value.span; let result = match &value.value { - Value::Number { .. } => self.value_node(node_id, "number", span, 1, vec![], 0), + Value::Number { .. } => self.value_node(node_id, "number", span, 2, vec![], 0), Value::String(_) => self.value_node(node_id, "string", span, 1, vec![], 0), Value::LocalizedString(_) => { self.value_node(node_id, "localized string", span, 2, vec![], 0) @@ -464,13 +452,14 @@ impl Counter<'_> { } Value::Enum { value_type, .. } => { let heroes = usize::from(value_type == "Hero"); - self.value_node(node_id, value_type, span, 1, vec![], heroes) + let base = if is_wrapped_enum(value_type) { 2 } else { 1 }; + self.value_node(node_id, value_type, span, base, vec![], heroes) } Value::GlobalVariable(_) => { - self.value_node(node_id, "global variable", span, 1, vec![], 0) + self.value_node(node_id, "global variable", span, 2, vec![], 0) } Value::PlayerVariable { player, .. } => { - self.value_children(node_id, "player variable", span, 1, &[*player]) + self.value_children(node_id, "player variable", span, 2, &[*player]) } Value::Subroutine(_) => self.value_node(node_id, "subroutine", span, 1, vec![], 0), Value::EventPlayer => self.value_node(node_id, "event player", span, 1, vec![], 0), @@ -501,16 +490,18 @@ impl Counter<'_> { } else { args.clone() }; - let base = if name == "array" - || name == "evalOnce" - || name.starts_with("workshopSetting") - || name.starts_with("createWorkshopSetting") - { + let setting_adjustment = workshop_setting_adjustment(name); + let base = if name == "customString" { + 1 + 4usize.saturating_sub(args.len()) + } else if is_comparison(name) { + 2 + } else if name == "array" || name == "evalOnce" { 2 } else { 1 }; - self.value_children(node_id, name, span, base, &child_ids) + let counted = self.value_children(node_id, name, span, base, &child_ids)?; + Ok(apply_adjustment(counted, setting_adjustment.unwrap_or(0))) } } }?; @@ -571,6 +562,53 @@ fn is_comparison(name: &str) -> bool { matches!(name, "==" | "!=" | "<" | "<=" | ">" | ">=") } +fn is_wrapped_enum(value_type: &str) -> bool { + matches!( + value_type, + "Button" | "Color" | "Gamemode" | "Hero" | "Map" | "Team" + ) +} + +fn workshop_setting_adjustment(name: &str) -> Option { + match name { + "createWorkshopSettingFloat" | "workshopSettingInteger" => Some(-3), + "workshopSettingCombo" => Some(-2), + "createWorkshopSettingHero" | "workshopSettingToggle" => Some(0), + _ => None, + } +} + +fn apply_adjustment(mut counted: Counted, adjustment: isize) -> Counted { + counted.node.adjustment += adjustment; + let children_count = counted + .node + .children + .iter() + .map(|child| child.count) + .sum::(); + counted.node.count = + (counted.node.base_count as isize + counted.node.adjustment + children_count as isize) + .max(0) as usize; + counted +} + +fn parameter_is_variable_reference(catalog: &Catalog, name: &str, index: usize) -> bool { + if name == "stopChasingPlayerVariable" && index == 0 { + return true; + } + catalog + .entry(Kind::Action, name) + .and_then(|entry| entry.param_type(index)) + .is_some_and(|types| { + types.split('|').any(|value_type| { + matches!( + value_type, + "Variable" | "Global Variable" | "Player Variable" + ) + }) + }) +} + fn is_canonical_helper(name: &str) -> bool { matches!( name, diff --git a/crates/workshop-rs/tests/element_count.rs b/crates/workshop-rs/tests/element_count.rs index 671cae6..bae7bad 100644 --- a/crates/workshop-rs/tests/element_count.rs +++ b/crates/workshop-rs/tests/element_count.rs @@ -5,7 +5,7 @@ use workshop_rs::catalog::{Catalog, Locale}; use workshop_rs::convert::{self, ConvertOptions}; use workshop_rs::parser; use workshop_rs::settings::{Settings, SettingsNode}; -use workshop_rs::{Action, Condition, Event, Program, Rule, Value, Variable}; +use workshop_rs::{Action, Event, Program, Rule, Value, Variable}; fn catalog() -> Catalog { Catalog::builtin().unwrap() @@ -64,8 +64,8 @@ fn arrays_localized_strings_and_hero_pairs_are_visible_in_the_tree() { let array_program = program_with_value(array); let array_report = array_program.element_count(&catalog()).unwrap(); assert_eq!( - array_report.total, 5, - "rule + action + (array 2 + literals 2 - top-level 1)" + array_report.total, 7, + "rule + action + (array 2 + numeric literals 4 - top-level 1)" ); let hero_program = program_with_value(Value::Array(vec![ @@ -79,7 +79,10 @@ fn arrays_localized_strings_and_hero_pairs_are_visible_in_the_tree() { }, ])); let hero_report = hero_program.element_count(&catalog()).unwrap(); - assert_eq!(hero_report.total, 6, "hero pair surcharge adds one element"); + assert_eq!( + hero_report.total, 8, + "hero wrappers and the pair surcharge add elements" + ); assert_eq!(hero_report.rules[0].children[0].adjustment, 1); let localized_report = program_with_value(Value::LocalizedString("hello".to_string())) @@ -92,13 +95,320 @@ fn arrays_localized_strings_and_hero_pairs_are_visible_in_the_tree() { } #[test] -fn custom_settings_and_disabled_rules_do_not_change_cost() { - let mut program = parser::parse( - "disabled rule (\"disabled\") { event { Ongoing - Global; } actions { Disable Inspector Recording; } }", - &catalog(), +fn wrapper_backed_enum_literals_count_the_wrapper() { + let report = program_with_value(Value::Enum { + value_type: "Map".to_string(), + value: "AATLIS".to_string(), + }) + .element_count(&catalog()) + .unwrap(); + + assert_eq!(report.total, 3, "rule + action + Map wrapper/literal"); + assert_eq!(report.rules[0].children[0].children[0].base_count, 2); +} + +#[test] +fn numeric_literals_and_player_variable_reads_match_reference_counts() { + let numeric_report = program_with_value(Value::number(1.0)) + .element_count(&catalog()) + .unwrap(); + assert_eq!(numeric_report.total, 3, "rule + action + numeric literal"); + let number = &numeric_report.rules[0].children[0].children[0]; + assert_eq!(number.base_count, 2); + assert_eq!(number.adjustment, -1); + + let program_with_global_value = |value| { + let mut program = Program::new(); + program + .global_variable(Variable::new("source")) + .global_variable(Variable::new("result")) + .rule( + Rule::new("read global", Event::Global).action(Action::SetGlobalVariable { + variable: "result".to_string(), + value, + }), + ); + program + }; + + let global_report = program_with_global_value(Value::GlobalVariable("source".to_string())) + .element_count(&catalog()) + .unwrap(); + assert_eq!(global_report.total, 3, "rule + action + global read"); + + let array_read = program_with_global_value(Value::Call { + name: "valueInArray".to_string(), + args: vec![ + Value::GlobalVariable("source".to_string()), + Value::number(12.0), + ], + }) + .element_count(&catalog()) + .unwrap(); + assert_eq!(array_read.total, 6, "rule + action + indexed global read"); + + let player_variable = |name| Value::player_variable(Value::EventPlayer, name); + let mut program = Program::new(); + for name in [ + "eventDurationHud", + "eventDuration", + "combatRegen", + "nanoEffect", + "hasNano", + ] { + program.player_variable(Variable::new(name)); + } + program.subroutine(workshop_rs::Subroutine::new("setEventDuration")); + program.rule( + Rule::new( + "duration", + Event::Subroutine("setEventDuration".to_string()), + ) + .action(Action::SetPlayerVariable { + player: Value::EventPlayer, + variable: "eventDurationHud".to_string(), + value: player_variable("eventDuration"), + }), + ); + program.rule( + Rule::new("regen", Event::EachPlayer) + .action(Action::call( + "skipIf", + [player_variable("combatRegen"), Value::Bool(true)], + )) + .action(Action::SetPlayerVariable { + player: Value::EventPlayer, + variable: "combatRegen".to_string(), + value: Value::Bool(true), + }), + ); + program.rule( + Rule::new("nano", Event::EachPlayer) + .action(Action::call( + "destroyEffect", + [player_variable("nanoEffect")], + )) + .action(Action::SetPlayerVariable { + player: Value::EventPlayer, + variable: "nanoEffect".to_string(), + value: Value::Null, + }) + .action(Action::SetPlayerVariable { + player: Value::EventPlayer, + variable: "hasNano".to_string(), + value: Value::Bool(false), + }), + ); + + let report = program.element_count(&catalog()).unwrap(); + assert_eq!( + report + .rule_counts() + .map(|(_, count)| count) + .collect::>(), + vec![4, 5, 6] + ); +} + +#[test] +fn custom_string_counts_unused_format_slots() { + let report = program_with_value(Value::Call { + name: "customString".to_string(), + args: vec![Value::String("abc".to_string())], + }) + .element_count(&catalog()) + .unwrap(); + + assert_eq!( + report.total, 6, + "rule + action + five-element custom string" + ); + let custom_string = &report.rules[0].children[0].children[0]; + assert_eq!(custom_string.base_count, 4); + assert_eq!(custom_string.children[0].count, 1); +} + +#[test] +fn workshop_setting_lowerings_match_pinned_overpy_counts() { + let integer_args = || { + vec![ + Value::String("category".to_string()), + Value::String("name".to_string()), + Value::number(1.0), + Value::number(0.0), + Value::number(10.0), + Value::number(1.0), + ] + }; + let cases = [ + ("workshopSettingInteger", integer_args(), 9, -4), + ("createWorkshopSettingFloat", integer_args(), 9, -4), + ( + "workshopSettingCombo", + vec![ + Value::String("category".to_string()), + Value::String("name".to_string()), + Value::number(0.0), + Value::Array(vec![ + Value::String("one".to_string()), + Value::String("two".to_string()), + ]), + Value::number(1.0), + ], + 10, + -3, + ), + ( + "workshopSettingToggle", + vec![ + Value::String("category".to_string()), + Value::String("name".to_string()), + Value::Bool(false), + Value::number(1.0), + ], + 7, + -1, + ), + ]; + + for (name, args, expected_total, expected_adjustment) in cases { + let report = program_with_value(Value::Call { + name: name.to_string(), + args, + }) + .element_count(&catalog()) + .unwrap(); + let setting = &report.rules[0].children[0].children[0]; + assert_eq!(report.total, expected_total, "{name}"); + assert_eq!(setting.base_count, 1, "{name}"); + assert_eq!(setting.adjustment, expected_adjustment, "{name}"); + } + + let hero_setting = program_with_value(Value::Call { + name: "createWorkshopSettingHero".to_string(), + args: vec![ + Value::String("category".to_string()), + Value::String("name".to_string()), + Value::Enum { + value_type: "Hero".to_string(), + value: "ANA".to_string(), + }, + Value::number(1.0), + ], + }) + .element_count(&catalog()) + .unwrap(); + let setting = &hero_setting.rules[0].children[0].children[0]; + assert_eq!(hero_setting.total, 8, "createWorkshopSettingHero"); + assert_eq!(setting.base_count, 1); + assert_eq!(setting.adjustment, -1); +} + +#[test] +fn structured_comparisons_and_control_markers_match_reference_counts() { + let catalog = catalog(); + let program = parser::parse( + r#"rule ("control") { event { Ongoing - Global; } actions { + If(Global.foo == 2); + Set Global Variable(foo, 0); + Else If(Global.foo == 3); + Set Global Variable(foo, 1); + Else; + Set Global Variable(foo, 2); + End; + While(Global.foo == 4); + Set Global Variable(foo, 3); + End; + } }"#, + &catalog, + &Locale::new("en-US"), + ) + .unwrap(); + + assert_eq!( + program.element_count(&catalog).unwrap().total, + 30, + "matches pinned OverPy 9.7.10 action annotations for this control flow" + ); + + let boolean_program = parser::parse( + r#"rule ("boolean control") { event { Ongoing - Global; } actions { + If(Or(Global.foo == 2, Global.foo == 3)); + Set Global Variable(foo, 0); + End; + } }"#, + &catalog, + &Locale::new("en-US"), + ) + .unwrap(); + assert_eq!( + boolean_program.element_count(&catalog).unwrap().total, + 17, + "matches pinned OverPy 9.7.10 nested comparison annotations" + ); + + let numeric_program = parser::parse( + r#"rule ("numeric comparison") { event { Ongoing - Global; } actions { + If(2 < 3); + Set Global Variable(foo, 0); + End; + } }"#, + &catalog, + &Locale::new("en-US"), + ) + .unwrap(); + assert_eq!( + numeric_program.element_count(&catalog).unwrap().total, + 10, + "numeric comparisons do not add a boolean projection" + ); +} + +#[test] +fn indexed_variable_targets_are_excluded_from_element_costs() { + let catalog = catalog(); + let program = parser::parse( + r#"variables { + global: 0: values + player: 0: state + } + rule ("indexed writes") { event { Ongoing - Each Player; } actions { + Set Global Variable At Index(values, false, 5); + Set Global Variable At Index(values, true, 6); + Modify Global Variable At Index(values, 2, Add, 7); + Set Player Variable At Index(Event Player, state, false, 5); + Set Player Variable At Index(Event Player, state, true, 6); + Modify Player Variable At Index(Event Player, state, 2, Add, 7); + Stop Chasing Player Variable(Event Player, state); + } }"#, + &catalog, &Locale::new("en-US"), ) .unwrap(); + + assert_eq!( + program.element_count(&catalog).unwrap().total, + 16, + "matches the seven pinned OverPy 9.7.10 action annotations" + ); +} + +#[test] +fn custom_settings_and_disabled_rules_actions_and_conditions_do_not_change_cost() { + let catalog = catalog(); + let locale = Locale::new("en-US"); + let active = parser::parse( + "rule (\"active\") { event { Ongoing - Global; } conditions { Is Game In Progress; } actions { Disable Inspector Recording; } }", + &catalog, + &locale, + ) + .unwrap(); + let mut program = parser::parse( + "disabled rule (\"disabled\") { event { Ongoing - Global; } conditions { disabled Is Game In Progress; } actions { disabled Disable Inspector Recording; } }", + &catalog, + &locale, + ) + .unwrap(); program.settings = Some(Settings { span: None, children: vec![SettingsNode::Raw { @@ -107,7 +417,11 @@ fn custom_settings_and_disabled_rules_do_not_change_cost() { span: None, }], }); - assert_eq!(program.element_count(&catalog()).unwrap().total, 2); + assert_eq!( + active.element_count(&catalog).unwrap().total, + program.element_count(&catalog).unwrap().total, + ); + assert_eq!(program.element_count(&catalog).unwrap().total, 3); } #[test] @@ -206,7 +520,7 @@ fn public_report_keeps_nested_values_and_actions_inspectable() { } #[test] -fn public_api_rejects_unsupported_and_invalid_programs_explicitly() { +fn public_api_rejects_unknown_actions_explicitly() { let mut unknown_action = Program::new(); unknown_action.rule( Rule::new("unknown", Event::Global) @@ -219,18 +533,6 @@ fn public_api_rejects_unsupported_and_invalid_programs_explicitly() { unknown_error, ElementCountError::InvalidProgram { message } if message.contains("unknown action") )); - - let mut invalid = Program::new(); - invalid.rule( - Rule::new("invalid", Event::Global).condition(Condition::disabled(Value::Bool(true))), - ); - let invalid_error = invalid - .element_count(&catalog()) - .expect_err("unsupported condition must not produce an exact count"); - assert!(matches!( - invalid_error, - ElementCountError::Unsupported { name, .. } if name == "disabled condition" - )); } fn assert_unique_node_ids(node: &workshop_rs::actions::ElementCountNode, ids: &mut HashSet) { diff --git a/docs/element-count.md b/docs/element-count.md index 0c6498c..6273ecb 100644 --- a/docs/element-count.md +++ b/docs/element-count.md @@ -16,52 +16,70 @@ aggregate total. Consumers should map nodes by their ordered tree position and source span when one is available; report-local IDs have no meaning across reports. -The model follows the documented Workshop element-count rules: +The model follows the Workshop element-count rules and the pinned OverPy 9.7.10 +element annotations: | Program component | Base cost | | --- | ---: | | Rule | 1 | | Action | 1 | | Condition | 1 | -| Ordinary value or literal | 1 | +| Ordinary value, boolean, null, or direct enum literal | 1 | +| Global-variable read | 2 | +| Wrapper-backed enum (`Button`, `Color`, `Gamemode`, `Hero`, `Map`, `Team`) | 2 | +| Number literal | 2 | +| Player-variable access | 2 plus its player expression | | Array | 2 | -| Workshop setting value (`Workshop Setting ...`) | 2 | +| Workshop Setting Integer or Real | 1, then subtract 3 | +| Workshop Setting Combo | 1, then subtract 2 | +| Workshop Setting Toggle | 1 | | Evaluate Once | 2 | | Localized/preset string | 2 | +`Custom String` uses the compiler's four argument slots. Its base cost is one, +plus one for each unused trailing slot, in addition to the text and supplied +value arguments. For example, `Custom String("abc")` costs five elements. + Rule event parameters, action syntax parameters such as a variable name or modify operator, comments, and custom game settings do not contribute. A -direct action or condition argument is reduced by one. For each pair of hero -literals anywhere below the direct arguments of one action or condition, one -element is added. Disabling a rule, action, or condition has no effect. +catalog action's `Variable`, `Global Variable`, or `Player Variable` target +parameter is syntax and is excluded. A direct action or condition argument is +reduced by one. A comparison value counts its operator syntax as a second base +element. `Else If`, `Else`, and `End` control markers each cost one element. +For each pair of hero literals anywhere below the direct arguments of one +action or condition, one element is added. Disabling a rule, action, or +condition has no effect. The calculator is locale-independent: it reads canonical identities and never emitted spellings. It validates the public program and catalog identities before producing a report. Unknown, unsupported, invalid, or cyclic constructs return `ElementCountError` instead of yielding a misleading exact total; no partial -report is returned. In -Native display actions such as `Create HUD Text` are counted through their +report is returned. Native display actions such as `Create HUD Text` are counted through their canonical catalog-backed action calls. Element count is a static structural Workshop complexity measure. It is not an estimate of runtime CPU cost or execution performance. -The independent behavioral source for the supported rules is the +The general rules follow the [Workshop.codes element-count calculation reference](https://workshop.codes/wiki/articles/element-count-calculation). +Construct-specific lowering is informed by pinned +[OverPy 9.7.10 element-count code](https://github.com/Zezombye/overpy/blob/v9.7.10/src/compiler/astToWorkshop.ts) +and checked against its per-rule and per-action annotations. The integration +tests preserve three small real-project rules with exact OverPy counts (4, 5, +and 6 elements), as well as representative numeric, variable, control-flow, +and setting expressions. Those checks establish compiler agreement for the +tested constructs, not independent client costs for each construct. -Known evidence gap: the model charges a numeric literal and `False`, `True`, or -`Null` the same cost, as the Workshop.codes reference groups them under one -literal cost. OverPy's `#!debugElementCount` charges a numeric literal argument -one element and `False`, `True`, or `Null` none, and its `#!optimizeForSize` -substitutions rely on that difference. No client capture establishes either -rule, so counts for programs that use these substitutions are unverified. -Parsing keeps the authored literal -([ADR-0015](adr/0015-contextual-literals-are-preserved.md)), so the analysis -can apply the rule a capture establishes. +For the Bastion OverPy build, the client capture is 30,070 elements, OverPy +reports 30,067, and this model reports 30,091 across 309 rules. The model is +within the stated 1% aggregate tolerance of the client (21 elements, about +0.07%). The client capture establishes the aggregate target; it does not +isolate individual construct costs. -This initial API does not claim live-client/editor -validation or source-language debug-count compatibility. Those belong to later -client-backed/consumer integration work after the canonical Program surface is -stable. The current real-project `rework.ow` fixture still stops in the parser -on an ambiguous bare `None` enum spelling, so it is not counted as a passing -real-program result until that independent parser gap is resolved. +This API counts the canonical program representation. Source-language debug +counts remain compiler-specific, and the Bastion comparison is evidence for +that real project and pinned compiler version rather than every possible +client/editor context. The current real-project `rework.ow` fixture still +stops in the parser on an ambiguous bare `None` enum spelling, so it is not +counted as a passing real-program result until that independent parser gap is +resolved. From b8d798688903e13560215029c5b039552586f65e Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sat, 26 Sep 2026 23:54:53 +0800 Subject: [PATCH 2/4] fix(element-count): satisfy clippy condition --- crates/workshop-rs/src/analysis/element_count.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/workshop-rs/src/analysis/element_count.rs b/crates/workshop-rs/src/analysis/element_count.rs index a01ecfd..2fb0be2 100644 --- a/crates/workshop-rs/src/analysis/element_count.rs +++ b/crates/workshop-rs/src/analysis/element_count.rs @@ -493,9 +493,7 @@ impl Counter<'_> { let setting_adjustment = workshop_setting_adjustment(name); let base = if name == "customString" { 1 + 4usize.saturating_sub(args.len()) - } else if is_comparison(name) { - 2 - } else if name == "array" || name == "evalOnce" { + } else if is_comparison(name) || name == "array" || name == "evalOnce" { 2 } else { 1 From 1da319320fa06e9eff1de24e2950b4739014729e Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sun, 27 Sep 2026 01:42:29 +0800 Subject: [PATCH 3/4] fix(element-count): retain player cost in variable targets --- .../workshop-rs/src/analysis/element_count.rs | 36 +++++++++++++++ crates/workshop-rs/tests/element_count.rs | 46 +++++++++++++++++++ docs/element-count.md | 17 +++---- 3 files changed, 91 insertions(+), 8 deletions(-) diff --git a/crates/workshop-rs/src/analysis/element_count.rs b/crates/workshop-rs/src/analysis/element_count.rs index 2fb0be2..ecbc454 100644 --- a/crates/workshop-rs/src/analysis/element_count.rs +++ b/crates/workshop-rs/src/analysis/element_count.rs @@ -391,6 +391,17 @@ impl Counter<'_> { name = action_name.as_str(); for (index, argument) in args.iter().enumerate() { if parameter_is_variable_reference(self.catalog, action_name, index) { + if let Some(player) = variable_reference_player_expression( + self.program, + self.catalog, + action_name, + index, + *argument, + ) { + let counted = self.value(player, true)?; + heroes += counted.heroes; + children.push(counted.node); + } continue; } let counted = self.value(*argument, true)?; @@ -607,6 +618,31 @@ fn parameter_is_variable_reference(catalog: &Catalog, name: &str, index: usize) }) } +fn variable_reference_player_expression( + program: &Program, + catalog: &Catalog, + name: &str, + index: usize, + argument: ValueId, +) -> Option { + let is_player_variable_parameter = name == "stopChasingPlayerVariable" && index == 0 + || catalog + .entry(Kind::Action, name) + .and_then(|entry| entry.param_type(index)) + .is_some_and(|types| { + types + .split('|') + .any(|value_type| value_type == "Player Variable") + }); + if !is_player_variable_parameter { + return None; + } + match &program.values.get(argument)?.value { + Value::PlayerVariable { player, .. } => Some(*player), + _ => None, + } +} + fn is_canonical_helper(name: &str) -> bool { matches!( name, diff --git a/crates/workshop-rs/tests/element_count.rs b/crates/workshop-rs/tests/element_count.rs index bae7bad..a7eb016 100644 --- a/crates/workshop-rs/tests/element_count.rs +++ b/crates/workshop-rs/tests/element_count.rs @@ -393,6 +393,52 @@ fn indexed_variable_targets_are_excluded_from_element_costs() { ); } +#[test] +fn variable_targets_keep_nontrivial_player_expression_costs() { + let catalog = catalog(); + let program = parser::parse( + r#"variables { + player: 0: state + } + rule ("nontrivial player target") { event { Ongoing - Global; } actions { + Stop Chasing Player Variable(First Of(All Players(All Teams)), state); + } } + rule ("indexed nontrivial player target") { event { Ongoing - Each Player; } actions { + Set Player Variable At Index(First Of(All Players(All Teams)), state, false, 5); + } }"#, + &catalog, + &Locale::new("en-US"), + ) + .unwrap(); + + let report = program.element_count(&catalog).unwrap(); + assert_eq!( + report + .rule_counts() + .map(|(_, count)| count) + .collect::>(), + vec![5, 6], + "the player expression is counted in stop-chasing and indexed targets" + ); + assert_eq!(report.total, 11); + let action = &report.rules[0].children[0]; + assert_eq!(action.name, "stopChasingPlayerVariable"); + assert_eq!( + action.children.len(), + 1, + "the variable target itself is syntax" + ); + assert_eq!(action.children[0].name, "firstOf"); + assert_eq!( + action.children[0].adjustment, -1, + "top-level argument reduction" + ); + let indexed_action = &report.rules[1].children[0]; + assert_eq!(indexed_action.name, "setPlayerVariableAtIndex"); + assert_eq!(indexed_action.children[0].name, "firstOf"); + assert_eq!(indexed_action.children[0].adjustment, -1); +} + #[test] fn custom_settings_and_disabled_rules_actions_and_conditions_do_not_change_cost() { let catalog = catalog(); diff --git a/docs/element-count.md b/docs/element-count.md index 6273ecb..a2edf77 100644 --- a/docs/element-count.md +++ b/docs/element-count.md @@ -41,11 +41,12 @@ plus one for each unused trailing slot, in addition to the text and supplied value arguments. For example, `Custom String("abc")` costs five elements. Rule event parameters, action syntax parameters such as a variable name or -modify operator, comments, and custom game settings do not contribute. A -catalog action's `Variable`, `Global Variable`, or `Player Variable` target -parameter is syntax and is excluded. A direct action or condition argument is -reduced by one. A comparison value counts its operator syntax as a second base -element. `Else If`, `Else`, and `End` control markers each cost one element. +modify operator, comments, and custom game settings do not contribute. In a +catalog action's variable target parameter, the variable name is syntax; a +`Player Variable` target still counts its player expression as a direct action +argument. A direct action or condition argument is reduced by one. A comparison +value counts its operator syntax as a second base element. `Else If`, `Else`, +and `End` control markers each cost one element. For each pair of hero literals anywhere below the direct arguments of one action or condition, one element is added. Disabling a rule, action, or condition has no effect. @@ -71,9 +72,9 @@ and setting expressions. Those checks establish compiler agreement for the tested constructs, not independent client costs for each construct. For the Bastion OverPy build, the client capture is 30,070 elements, OverPy -reports 30,067, and this model reports 30,091 across 309 rules. The model is -within the stated 1% aggregate tolerance of the client (21 elements, about -0.07%). The client capture establishes the aggregate target; it does not +reports 30,067, and this model reports 30,095 across 309 rules. The model is +within the stated 1% aggregate tolerance of the client (25 elements, about +0.08%). The client capture establishes the aggregate target; it does not isolate individual construct costs. This API counts the canonical program representation. Source-language debug From 943a06bacd4c6afa3f4d78a4c3f6b86e56c038d4 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sun, 27 Sep 2026 01:48:27 +0800 Subject: [PATCH 4/4] style(test): format player target cost assertion --- crates/workshop-rs/tests/element_count.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/workshop-rs/tests/element_count.rs b/crates/workshop-rs/tests/element_count.rs index 66b491e..7a453d8 100644 --- a/crates/workshop-rs/tests/element_count.rs +++ b/crates/workshop-rs/tests/element_count.rs @@ -267,7 +267,10 @@ fn player_variable_targets_count_nontrivial_player_expressions() { let report = program.element_count(&catalog).unwrap(); assert_eq!( report.rule_counts().collect::>(), - vec![("player-variable chase", 5), ("indexed player-variable target", 6)], + vec![ + ("player-variable chase", 5), + ("indexed player-variable target", 6) + ], "both target forms count the player expression with the direct-argument reduction" ); assert_eq!(report.total, 11);