From 943741617957f6fecd11ba184870005a0f57bf3e Mon Sep 17 00:00:00 2001 From: Teakowa Date: Mon, 24 Aug 2026 13:35:00 +0800 Subject: [PATCH] feat(workshop): add canonical min/max variable modifiers Fixes #95 --- crates/workshop-rs-cli/tests/cli.rs | 2 +- .../workshop-rs/src/catalog/data/catalog.json | 16 +- crates/workshop-rs/src/element_count.rs | 2 + crates/workshop-rs/src/emitter.rs | 15 +- crates/workshop-rs/src/parser.rs | 151 +++++++++++++----- crates/workshop-rs/src/semantic.rs | 2 + crates/workshop-rs/src/validate.rs | 4 + crates/workshop-rs/src/wir/dump.rs | 2 + crates/workshop-rs/src/wir/mod.rs | 21 +++ crates/workshop-rs/tests/catalog.rs | 16 ++ crates/workshop-rs/tests/emitter.rs | 27 ++++ crates/workshop-rs/tests/identity.rs | 2 +- crates/workshop-rs/tests/parser.rs | 88 ++++++++++ docs/language-support/operators.md | 2 + 14 files changed, 299 insertions(+), 51 deletions(-) diff --git a/crates/workshop-rs-cli/tests/cli.rs b/crates/workshop-rs-cli/tests/cli.rs index c6aeed7..a4f928c 100644 --- a/crates/workshop-rs-cli/tests/cli.rs +++ b/crates/workshop-rs-cli/tests/cli.rs @@ -137,7 +137,7 @@ fn locales_lists_declared_locales_with_coverage() { assert_eq!(lines.len(), 2); for (line, (locale, expected)) in lines .iter() - .zip([("en-us", None), ("zh-cn", Some(("1240", "1259")))]) + .zip([("en-us", None), ("zh-cn", Some(("1242", "1261")))]) { let (reported_locale, coverage) = line.split_once(' ').expect("locale coverage line"); let (mapped, total) = coverage.split_once('/').expect("mapped/total coverage"); diff --git a/crates/workshop-rs/src/catalog/data/catalog.json b/crates/workshop-rs/src/catalog/data/catalog.json index 957df9b..c130608 100644 --- a/crates/workshop-rs/src/catalog/data/catalog.json +++ b/crates/workshop-rs/src/catalog/data/catalog.json @@ -3992,7 +3992,7 @@ ] } ], - "digest": "174a4ebc3f1250485ea55605ebb885016d84897368f65f1ad3818f9789728060", + "digest": "e3a0c15bd30fd59ec933114bccce5feaeb5c8a77bb34497b185e5a36dab41d34", "enums": [ { "domain": "Impulse", @@ -9658,6 +9658,20 @@ }, "id": "modulo" }, + { + "aliases": { + "en-US": "Min", + "zh-CN": "较小" + }, + "id": "min" + }, + { + "aliases": { + "en-US": "Max", + "zh-CN": "较大" + }, + "id": "max" + }, { "aliases": { "en-US": "Raise To Power", diff --git a/crates/workshop-rs/src/element_count.rs b/crates/workshop-rs/src/element_count.rs index 3ee76d0..b77b9e1 100644 --- a/crates/workshop-rs/src/element_count.rs +++ b/crates/workshop-rs/src/element_count.rs @@ -538,6 +538,8 @@ fn is_canonical_helper(name: &str) -> bool { | "multiply" | "divide" | "modulo" + | "min" + | "max" | "raiseToPower" | "appendToArray" | "removeFromArray" diff --git a/crates/workshop-rs/src/emitter.rs b/crates/workshop-rs/src/emitter.rs index 48804d9..d6ebb3a 100644 --- a/crates/workshop-rs/src/emitter.rs +++ b/crates/workshop-rs/src/emitter.rs @@ -835,6 +835,8 @@ impl Emitter<'_> { wir::ModifyOp::Multiply => "*", wir::ModifyOp::Divide => "/", wir::ModifyOp::Modulo => "%", + wir::ModifyOp::Min => "min", + wir::ModifyOp::Max => "max", _ => { return Err(WorkshopError::Unsupported { message: format!( @@ -1462,18 +1464,7 @@ impl Emitter<'_> { /// The localized spelling of a modify operator, resolved through the /// catalog (fallback-aware). fn modify_op_spelling(&mut self, op: wir::ModifyOp) -> Result { - let id = match op { - wir::ModifyOp::Add => "add", - wir::ModifyOp::Subtract => "subtract", - wir::ModifyOp::Multiply => "multiply", - wir::ModifyOp::Divide => "divide", - wir::ModifyOp::Modulo => "modulo", - wir::ModifyOp::RaiseToPower => "raiseToPower", - wir::ModifyOp::AppendToArray => "appendToArray", - wir::ModifyOp::RemoveFromArray => "removeFromArray", - wir::ModifyOp::RemoveFromArrayByIndex => "removeFromArrayByIndex", - }; - self.spelling(Kind::Operator, id) + self.spelling(Kind::Operator, op.catalog_id()) } /// The localized spelling of a canonical builtin id, resolving through diff --git a/crates/workshop-rs/src/parser.rs b/crates/workshop-rs/src/parser.rs index 2a43dce..9435bfb 100644 --- a/crates/workshop-rs/src/parser.rs +++ b/crates/workshop-rs/src/parser.rs @@ -1376,7 +1376,7 @@ impl Parser<'_> { TokenKind::RBracket, "expected ']' after global variable index", )?; - let _operator = self.assignment_operator().ok_or_else(|| { + let operator = self.assignment_operator()?.ok_or_else(|| { self.malformed( "expected assignment after global variable index", self.peek().as_ref().unwrap_or(self.eof()), @@ -1389,13 +1389,11 @@ impl Parser<'_> { )); let value = self.value()?; self.expect(TokenKind::Semi, "expected ';' after indexed assignment")?; - return Ok(Some(self.target.actions.push(Action::Call { - name: "setGlobalVariableAtIndex".to_string(), - args: vec![target, index, value], - span: Some(Span::new(self.file(), start, self.previous_span().1)), - }))); + return Ok(Some(self.indexed_assignment_action( + true, target, index, operator, value, start, + ))); } - let Some(operator) = self.assignment_operator() else { + let Some(operator) = self.assignment_operator()? else { return self.member_assignment_action(saved, start); }; let variable = self.global_by_name(&name)?; @@ -1463,7 +1461,7 @@ impl Parser<'_> { TokenKind::RBracket, "expected ']' after player variable index", )?; - let _operator = self.assignment_operator().ok_or_else(|| { + let operator = self.assignment_operator()?.ok_or_else(|| { self.malformed( "expected assignment after player variable index", self.peek().as_ref().unwrap_or(self.eof()), @@ -1478,13 +1476,16 @@ impl Parser<'_> { }, Some(Span::new(self.file(), target_start, target_end)), )); - return Ok(Some(self.target.actions.push(Action::Call { - name: "setPlayerVariableAtIndex".to_string(), - args: vec![variable_value, index, value], - span: Some(Span::new(self.file(), start, self.previous_span().1)), - }))); + return Ok(Some(self.indexed_assignment_action( + false, + variable_value, + index, + operator, + value, + start, + ))); } - let Some(operator) = self.assignment_operator() else { + let Some(operator) = self.assignment_operator()? else { return self.member_assignment_action(saved, start); }; let value = self.value()?; @@ -1520,7 +1521,7 @@ impl Parser<'_> { return Ok(None); } let target = self.value()?; - let Some(operator) = self.assignment_operator() else { + let Some(operator) = self.assignment_operator()? else { self.pos = saved; return Ok(None); }; @@ -1538,14 +1539,88 @@ impl Parser<'_> { }))) } - fn assignment_operator(&mut self) -> Option { - let operator = match self.peek()?.kind { - TokenKind::Op(operator) => operator, - _ => return None, + fn indexed_assignment_action( + &mut self, + global: bool, + variable: wir::ValueId, + index: wir::ValueId, + operator: AssignmentOperator, + value: wir::ValueId, + start: Position, + ) -> wir::ActionId { + let (name, args) = match operator { + AssignmentOperator::Set => ( + if global { + "setGlobalVariableAtIndex" + } else { + "setPlayerVariableAtIndex" + }, + vec![variable, index, value], + ), + AssignmentOperator::Modify(op) => ( + if global { + "modifyGlobalVariableAtIndex" + } else { + "modifyPlayerVariableAtIndex" + }, + vec![ + variable, + index, + self.target.values.push(ValueNode::new( + Value::Call { + name: op.catalog_id().to_string(), + args: Vec::new(), + }, + None, + )), + value, + ], + ), + }; + self.target.actions.push(Action::Call { + name: name.to_string(), + args, + span: Some(Span::new(self.file(), start, self.previous_span().1)), + }) + } + + fn assignment_operator(&mut self) -> Result> { + let Some(token) = self.peek() else { + return Ok(None); + }; + if let TokenKind::Word(word) = &token.kind { + let op = match word.as_str() { + "min" => ModifyOp::Min, + "max" => ModifyOp::Max, + _ => { + if matches!(self.peek_at(1).map(|token| token.kind), Some(TokenKind::Op(equal)) if equal == "=") + { + return Err(WorkshopError::Unsupported { + message: format!("unsupported assignment operator '{word}='"), + span: Some(Span::new( + self.file(), + token.start, + self.peek_at(1).unwrap().end, + )), + }); + } + return Ok(None); + } + }; + if !matches!(self.peek_at(1).map(|token| token.kind), Some(TokenKind::Op(equal)) if equal == "=") + { + return Ok(None); + } + self.pos += 2; + return Ok(Some(AssignmentOperator::Modify(op))); + } + let operator = match &token.kind { + TokenKind::Op(operator) => operator.clone(), + _ => return Ok(None), }; if operator == "=" { self.pos += 1; - return Some(AssignmentOperator::Set); + return Ok(Some(AssignmentOperator::Set)); } let op = match operator.as_str() { "+" => ModifyOp::Add, @@ -1553,14 +1628,14 @@ impl Parser<'_> { "*" => ModifyOp::Multiply, "/" => ModifyOp::Divide, "%" => ModifyOp::Modulo, - _ => return None, + _ => return Ok(None), }; if !matches!(self.peek_at(1).map(|token| token.kind), Some(TokenKind::Op(equal)) if equal == "=") { - return None; + return Ok(None); } self.pos += 2; - Some(AssignmentOperator::Modify(op)) + Ok(Some(AssignmentOperator::Modify(op))) } fn opaque_action(&mut self) -> Result { @@ -2015,6 +2090,8 @@ impl Parser<'_> { "multiply" => ModifyOp::Multiply, "divide" => ModifyOp::Divide, "modulo" => ModifyOp::Modulo, + "min" => ModifyOp::Min, + "max" => ModifyOp::Max, "raiseToPower" => ModifyOp::RaiseToPower, "appendToArray" => ModifyOp::AppendToArray, "removeFromArray" | "removeFromArrayByValue" => ModifyOp::RemoveFromArray, @@ -2911,17 +2988,7 @@ impl Parser<'_> { is_operator } { let operator = self.modify_op()?; - let name = match operator { - ModifyOp::Add => "add", - ModifyOp::Subtract => "subtract", - ModifyOp::Multiply => "multiply", - ModifyOp::Divide => "divide", - ModifyOp::Modulo => "modulo", - ModifyOp::RaiseToPower => "raiseToPower", - ModifyOp::AppendToArray => "appendToArray", - ModifyOp::RemoveFromArray => "removeFromArray", - ModifyOp::RemoveFromArrayByIndex => "removeFromArrayByIndex", - }; + let name = operator.catalog_id(); args.push(self.target.values.push(ValueNode::new( Value::Call { name: name.to_string(), @@ -3104,10 +3171,16 @@ impl Parser<'_> { } fn line_has_assignment(&self) -> bool { - self.tokens[self.pos..] + let tokens: Vec<_> = self.tokens[self.pos..] .iter() .take_while(|token| !matches!(token.kind, TokenKind::Semi | TokenKind::RBrace)) - .any(|token| matches!(&token.kind, TokenKind::Op(op) if matches!(op.as_str(), "=" | "+=" | "-=" | "*=" | "/=" | "%="))) + .collect(); + tokens.iter().any(|token| { + matches!(&token.kind, TokenKind::Op(op) if matches!(op.as_str(), "=" | "+=" | "-=" | "*=" | "/=" | "%=")) + }) || tokens.windows(2).any(|window| { + matches!(&window[0].kind, TokenKind::Word(_)) + && matches!(&window[1].kind, TokenKind::Op(op) if op == "=") + }) } fn push_bool(&mut self, value: bool, start: Position, end: Position) -> wir::ValueId { @@ -3204,6 +3277,12 @@ impl Parser<'_> { end: word_end, .. } => { + if matches!( + self.peek_at(1).map(|token| token.kind), + Some(TokenKind::Op(equal)) if equal == "=" + ) { + break; + } words.push(word.clone()); end = word_end; self.pos += 1; diff --git a/crates/workshop-rs/src/semantic.rs b/crates/workshop-rs/src/semantic.rs index de1cbc9..63b5df7 100644 --- a/crates/workshop-rs/src/semantic.rs +++ b/crates/workshop-rs/src/semantic.rs @@ -222,6 +222,8 @@ fn inspect_value( | "multiply" | "divide" | "modulo" + | "min" + | "max" | "raiseToPower" | "appendToArray" | "removeFromArray" diff --git a/crates/workshop-rs/src/validate.rs b/crates/workshop-rs/src/validate.rs index 0606745..db862cf 100644 --- a/crates/workshop-rs/src/validate.rs +++ b/crates/workshop-rs/src/validate.rs @@ -230,6 +230,8 @@ fn validate_value( | "multiply" | "divide" | "modulo" + | "min" + | "max" | "raiseToPower" | "appendToArray" | "removeFromArray" @@ -476,6 +478,8 @@ fn value_matches_single_type(catalog: &Catalog, value: &wir::Value, expected: &s | "multiply" | "divide" | "modulo" + | "min" + | "max" | "raiseToPower" | "appendToArray" | "removeFromArray" diff --git a/crates/workshop-rs/src/wir/dump.rs b/crates/workshop-rs/src/wir/dump.rs index ea75f69..310bb66 100644 --- a/crates/workshop-rs/src/wir/dump.rs +++ b/crates/workshop-rs/src/wir/dump.rs @@ -206,6 +206,8 @@ fn render_action(program: &Program, id: super::ActionId, out: &mut String, level Some(ModifyOp::Multiply) => " *= ", Some(ModifyOp::Divide) => " /= ", Some(ModifyOp::Modulo) => " %= ", + Some(ModifyOp::Min) => " min= ", + Some(ModifyOp::Max) => " max= ", Some(_) => " ", }); render_value(program, *value, out); diff --git a/crates/workshop-rs/src/wir/mod.rs b/crates/workshop-rs/src/wir/mod.rs index 59c6233..7520cde 100644 --- a/crates/workshop-rs/src/wir/mod.rs +++ b/crates/workshop-rs/src/wir/mod.rs @@ -450,6 +450,8 @@ pub enum ModifyOp { Multiply, Divide, Modulo, + Min, + Max, RaiseToPower, AppendToArray, RemoveFromArray, @@ -465,10 +467,29 @@ impl ModifyOp { ModifyOp::Multiply => "Multiply", ModifyOp::Divide => "Divide", ModifyOp::Modulo => "Modulo", + ModifyOp::Min => "Min", + ModifyOp::Max => "Max", ModifyOp::RaiseToPower => "RaiseToPower", ModifyOp::AppendToArray => "AppendToArray", ModifyOp::RemoveFromArray => "RemoveFromArray", ModifyOp::RemoveFromArrayByIndex => "RemoveFromArrayByIndex", } } + + /// The canonical catalog identity for this modification operation. + pub fn catalog_id(self) -> &'static str { + match self { + ModifyOp::Add => "add", + ModifyOp::Subtract => "subtract", + ModifyOp::Multiply => "multiply", + ModifyOp::Divide => "divide", + ModifyOp::Modulo => "modulo", + ModifyOp::Min => "min", + ModifyOp::Max => "max", + ModifyOp::RaiseToPower => "raiseToPower", + ModifyOp::AppendToArray => "appendToArray", + ModifyOp::RemoveFromArray => "removeFromArray", + ModifyOp::RemoveFromArrayByIndex => "removeFromArrayByIndex", + } + } } diff --git a/crates/workshop-rs/tests/catalog.rs b/crates/workshop-rs/tests/catalog.rs index 4d42810..20d1c8c 100644 --- a/crates/workshop-rs/tests/catalog.rs +++ b/crates/workshop-rs/tests/catalog.rs @@ -358,6 +358,22 @@ fn documented_action_and_value_signatures_are_inventory_entries() { assert_eq!(array.param_type(3), Some("Object|Array")); } +#[test] +fn min_max_are_canonical_operator_identities() { + let catalog = builtin(); + for (id, en_spelling, zh_spelling) in [("min", "Min", "较小"), ("max", "Max", "较大")] { + let entry = catalog.entry(Kind::Operator, id).expect(id); + assert_eq!(entry.spelling(&en()), Some(en_spelling)); + assert_eq!(entry.spelling(&Locale::new("zh-CN")), Some(zh_spelling)); + assert_eq!( + catalog + .resolve(Kind::Operator, &en(), en_spelling) + .map(|entry| entry.id.as_str()), + Some(id) + ); + } +} + #[test] fn exercised_enum_domains_resolve_members_to_canonical_identity() { let catalog = builtin(); diff --git a/crates/workshop-rs/tests/emitter.rs b/crates/workshop-rs/tests/emitter.rs index f35f5a4..0bede97 100644 --- a/crates/workshop-rs/tests/emitter.rs +++ b/crates/workshop-rs/tests/emitter.rs @@ -129,6 +129,33 @@ fn event_player_member_assignment_emits_action_reparsable_syntax() { assert!(workshop_rs::roundtrip::equivalent(&program, &reparsed)); } +#[test] +fn min_max_operations_emit_and_round_trip_in_zh_cn() { + let source = r#" + variables { + global: 0: g + player: 0: p + } + rule ("min-max") { + event { Ongoing - Global; } + actions { + Modify Global Variable(g, Min, 1); + Modify Player Variable(Event Player, p, Max, 2); + Modify Global Variable At Index(g, 0, Min, 3); + Modify Player Variable At Index(Event Player, p, 1, Max, 4); + } + } + "#; + let catalog = catalog(); + let program = parser::parse_with_context(source, &catalog, &en(), &catalog).unwrap(); + let emitted = emitter::emit(&program, &catalog, &Locale::new("zh-CN")).unwrap(); + assert!(emitted.contains("较小"), "{emitted}"); + assert!(emitted.contains("较大"), "{emitted}"); + let reparsed = + parser::parse_with_context(&emitted, &catalog, &Locale::new("zh-CN"), &catalog).unwrap(); + assert!(workshop_rs::roundtrip::equivalent(&program, &reparsed)); +} + #[test] fn every_corpus_program_round_trips_to_equivalent_wir() { // Corpus text parses against the catalog context (expected enum domains diff --git a/crates/workshop-rs/tests/identity.rs b/crates/workshop-rs/tests/identity.rs index 32befac..7a77152 100644 --- a/crates/workshop-rs/tests/identity.rs +++ b/crates/workshop-rs/tests/identity.rs @@ -12,7 +12,7 @@ use workshop_rs::catalog::{Catalog, Locale}; /// (`workshop-catalog-gen build`) recomputes it and the pin is updated /// deliberately together with the data. const PINNED_CATALOG_DIGEST: &str = - "174a4ebc3f1250485ea55605ebb885016d84897368f65f1ad3818f9789728060"; + "e3a0c15bd30fd59ec933114bccce5feaeb5c8a77bb34497b185e5a36dab41d34"; #[test] fn committed_catalog_digest_is_pinned() { diff --git a/crates/workshop-rs/tests/parser.rs b/crates/workshop-rs/tests/parser.rs index 33311b1..a90ede1 100644 --- a/crates/workshop-rs/tests/parser.rs +++ b/crates/workshop-rs/tests/parser.rs @@ -769,6 +769,94 @@ fn raw_indexed_assignment_lowers_to_explicit_wir_call() { .expect("the indexed assignment must validate"); } +#[test] +fn min_max_modifications_lower_for_global_player_and_indexed_forms() { + // OverPy's canonical min=/max= forms and the raw Workshop actions both + // denote the same Workshop modification operation (workshop-rs#95). + let text = r#" + variables { + global: 0: g + player: 0: p + } + rule ("min-max") { + event { Ongoing - Global; } + actions { + Global.g min= 1; + Event Player.p max= 2; + Global.g[0] min= 3; + Event Player.p[1] max= 4; + Modify Global Variable(g, Min, 5); + Modify Player Variable(Event Player, p, Max, 6); + Modify Global Variable At Index(g, 0, Min, 7); + Modify Player Variable At Index(Event Player, p, 1, Max, 8); + } + } + "#; + let catalog = catalog(); + let program = parser::parse_with_context(text, &catalog, &Locale::new("en-US"), &catalog) + .expect("min/max forms parse"); + validate::validate_canonical_ids(&program, &catalog).expect("min/max ids validate"); + + let direct: Vec<_> = program + .actions + .iter() + .filter_map(|action| match action { + wir::Action::ModifyGlobalVariable { op, .. } + | wir::Action::ModifyPlayerVariable { op, .. } => Some(*op), + _ => None, + }) + .collect(); + assert_eq!( + direct, + [ + wir::ModifyOp::Min, + wir::ModifyOp::Max, + wir::ModifyOp::Min, + wir::ModifyOp::Max, + ] + ); + + let indexed: Vec<_> = program + .actions + .iter() + .filter_map(|action| match action { + wir::Action::Call { name, args, .. } + if matches!( + name.as_str(), + "modifyGlobalVariableAtIndex" | "modifyPlayerVariableAtIndex" + ) => + { + Some((name.as_str(), args[2])) + } + _ => None, + }) + .collect(); + assert_eq!(indexed.len(), 4); + assert!(indexed.iter().all(|(_, id)| matches!( + program.values.get(*id), + Some(wir::ValueNode { + value: wir::Value::Call { name, args }, + .. + }) if args.is_empty() && matches!(name.as_str(), "min" | "max") + ))); +} + +#[test] +fn unsupported_named_assignment_operator_is_rejected() { + let text = r#" + rule ("unsupported") { + event { Ongoing - Global; } + actions { Global.g median= 1; } + } + "#; + let error = parser::parse_with_context(text, &catalog(), &Locale::new("en-US"), &catalog()) + .expect_err("unsupported named assignment must fail"); + assert!(matches!( + error, + workshop_rs::WorkshopError::Unsupported { .. } + )); +} + #[test] fn cross_domain_member_spelling_collisions_are_the_documented_inventory() { // Systematic collision check: scan the declared catalog for member diff --git a/docs/language-support/operators.md b/docs/language-support/operators.md index 49b12d3..435d3ee 100644 --- a/docs/language-support/operators.md +++ b/docs/language-support/operators.md @@ -21,6 +21,8 @@ | `Append To Array` | ✅ Supported | Array variable modification operation. | | `Divide` | ✅ Supported | Arithmetic operator and variable modification operation. | | `Modulo` | ✅ Supported | Arithmetic operator and variable modification operation. | +| `Min` | ✅ Supported | Variable modification operation that clamps the variable to the lower value. | +| `Max` | ✅ Supported | Variable modification operation that clamps the variable to the higher value. | | `Multiply` | ✅ Supported | Arithmetic operator and variable modification operation. | | `Raise To Power` | ✅ Supported | Arithmetic operator and variable modification operation. | | `Remove From Array` | ✅ Supported | Array variable modification operation. |