Skip to content

Commit 230bc16

Browse files
fix(PR-6192): preserve nested keyword choices as unsupported
Route the parser's strict nested-choice outcome through the subject AST so imperative fallback cannot reinterpret it as fixed keyword grants. Co-authored-by: Try everything <tryeverything24@users.noreply.github.com>
1 parent 4aceb04 commit 230bc16

1 file changed

Lines changed: 55 additions & 31 deletions

File tree

crates/engine/src/parser/oracle_effect/subject.rs

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -142,17 +142,35 @@ pub(super) fn try_parse_subject_predicate_ast(
142142
return Some(clause);
143143
}
144144

145-
if let Some(clause) = try_parse_subject_continuous_clause(text, ctx) {
146-
return Some(subject_predicate_ast_from_clause(
147-
text,
148-
clause,
149-
|effect, duration, sub_ability| PredicateAst::Continuous {
150-
effect,
151-
duration,
152-
sub_ability,
153-
},
154-
ctx,
155-
));
145+
match try_parse_subject_continuous_clause(text, ctx) {
146+
Ok(Some(clause)) => {
147+
return Some(subject_predicate_ast_from_clause(
148+
text,
149+
clause,
150+
|effect, duration, sub_ability| PredicateAst::Continuous {
151+
effect,
152+
duration,
153+
sub_ability,
154+
},
155+
ctx,
156+
));
157+
}
158+
Ok(None) => {}
159+
// `parse_keyword_choice_grant` recognized an outer menu containing an
160+
// inner player choice. Preserve that unsupported semantic instead of
161+
// allowing the imperative fallback to reinterpret it as fixed keywords.
162+
Err(()) => {
163+
return Some(subject_predicate_ast_from_clause(
164+
text,
165+
super::parsed_clause(Effect::unimplemented("nested keyword choice", text)),
166+
|effect, duration, sub_ability| PredicateAst::Continuous {
167+
effect,
168+
duration,
169+
sub_ability,
170+
},
171+
ctx,
172+
));
173+
}
156174
}
157175

158176
// CR 613.4b + CR 613.1f: "[subject]'s base power and toughness become N/M and
@@ -469,8 +487,10 @@ fn try_parse_contracted_subject_additive_type_clause(
469487
fn try_parse_subject_continuous_clause(
470488
text: &str,
471489
ctx: &mut ParseContext,
472-
) -> Option<ParsedEffectClause> {
473-
let verb_start = find_predicate_start(text)?;
490+
) -> Result<Option<ParsedEffectClause>, ()> {
491+
let Some(verb_start) = find_predicate_start(text) else {
492+
return Ok(None);
493+
};
474494
// CR 608.2c: An additive "also" sitting between a filter subject and its
475495
// continuous verb ("Kithkin creatures you control *also* gain first strike
476496
// until end of turn") is a natural-language connector with no semantic
@@ -489,12 +509,14 @@ fn try_parse_subject_continuous_clause(
489509
// the continuous arm misclassifies imperatives like "you get an emblem
490510
// with \"…\"" as `get +X/+X`-style P/T modifications.
491511
if subject.eq_ignore_ascii_case("you") {
492-
return None;
512+
return Ok(None);
493513
}
494514
if let Some(clause) = try_parse_additive_type_continuous_clause(subject, predicate, ctx) {
495-
return Some(clause);
515+
return Ok(Some(clause));
496516
}
497-
let application = parse_subject_application(subject, ctx)?;
517+
let Some(application) = parse_subject_application(subject, ctx) else {
518+
return Ok(None);
519+
};
498520
build_continuous_clause(application, predicate, ctx)
499521
}
500522

@@ -3527,7 +3549,7 @@ fn build_continuous_clause(
35273549
application: SubjectApplication,
35283550
predicate: &str,
35293551
ctx: &ParseContext,
3530-
) -> Option<ParsedEffectClause> {
3552+
) -> Result<Option<ParsedEffectClause>, ()> {
35313553
let normalized = deconjugate_verb(predicate);
35323554

35333555
// B15: Guard against "becomes" predicates routing through continuous clause parsing.
@@ -3537,21 +3559,21 @@ fn build_continuous_clause(
35373559
.parse(normalized.as_str())
35383560
.is_ok()
35393561
{
3540-
return None;
3562+
return Ok(None);
35413563
}
35423564
if tag::<_, _, OracleError<'_>>("create ")
35433565
.parse(normalized.as_str())
35443566
.is_ok()
35453567
{
3546-
return None;
3568+
return Ok(None);
35473569
}
35483570

35493571
// Try the full predicate first (simple pump with no compound).
35503572
if let Some((power, toughness, duration)) =
35513573
super::lower::parse_pump_clause_with_context(&normalized, ctx)
35523574
{
35533575
let effect = build_pump_effect(&application, power, toughness);
3554-
return Some(ParsedEffectClause {
3576+
return Ok(Some(ParsedEffectClause {
35553577
effect,
35563578
duration,
35573579
sub_ability: None,
@@ -3560,23 +3582,23 @@ fn build_continuous_clause(
35603582
condition: None,
35613583
optional: false,
35623584
unless_pay: None,
3563-
});
3585+
}));
35643586
}
35653587

35663588
// Compound: "get +1/+1 until end of turn and you gain 1 life"
35673589
// Split on " and " that follows a duration marker, producing a pump
35683590
// with a chained sub_ability for the remainder.
35693591
if let Some(clause) = try_split_pump_compound(&normalized, &application, ctx) {
3570-
return Some(clause);
3592+
return Ok(Some(clause));
35713593
}
35723594

35733595
match build_keyword_choice_clause(&application, &normalized) {
3574-
Ok(Some(clause)) => return Some(clause),
3596+
Ok(Some(clause)) => return Ok(Some(clause)),
35753597
Ok(None) => {}
35763598
// A nested keyword choice cannot be represented by a fixed AddKeyword.
35773599
// Do not fall through to generic static parsing, which would turn the
35783600
// menu into simultaneous grants and falsely mark the card supported.
3579-
Err(()) => return None,
3601+
Err(()) => return Err(()),
35803602
}
35813603

35823604
// Strip "where X is..." and "for each..." suffixes before extracting duration,
@@ -3595,7 +3617,7 @@ fn build_continuous_clause(
35953617
if let Some(static_abilities) =
35963618
build_defender_attack_continuous_compound(&application, predicate_text)
35973619
{
3598-
return Some(ParsedEffectClause {
3620+
return Ok(Some(ParsedEffectClause {
35993621
effect: Effect::GenericEffect {
36003622
static_abilities,
36013623
duration: duration.clone(),
@@ -3608,12 +3630,12 @@ fn build_continuous_clause(
36083630
condition: None,
36093631
optional: false,
36103632
unless_pay: None,
3611-
});
3633+
}));
36123634
}
36133635

36143636
let modifications = parse_continuous_modifications(predicate_text);
36153637
if modifications.is_empty() {
3616-
return None;
3638+
return Ok(None);
36173639
}
36183640

36193641
// CR 702.62b + CR 611.2a + CR 611.2c: A "gains suspend" grant onto an exiled
@@ -3638,7 +3660,7 @@ fn build_continuous_clause(
36383660

36393661
if let Some((power, toughness)) = extract_pump_modifiers(&modifications) {
36403662
let effect = build_pump_effect(&application, power, toughness);
3641-
return Some(ParsedEffectClause {
3663+
return Ok(Some(ParsedEffectClause {
36423664
effect,
36433665
duration,
36443666
sub_ability: None,
@@ -3647,7 +3669,7 @@ fn build_continuous_clause(
36473669
condition: None,
36483670
optional: false,
36493671
unless_pay: None,
3650-
});
3672+
}));
36513673
}
36523674

36533675
let affected = static_affected_for_application(&application);
@@ -3674,7 +3696,7 @@ fn build_continuous_clause(
36743696
.description(predicate_text.to_string())]
36753697
};
36763698

3677-
Some(ParsedEffectClause {
3699+
Ok(Some(ParsedEffectClause {
36783700
effect: Effect::GenericEffect {
36793701
static_abilities,
36803702
duration: duration.clone(),
@@ -3687,7 +3709,7 @@ fn build_continuous_clause(
36873709
condition: None,
36883710
optional: false,
36893711
unless_pay: None,
3690-
})
3712+
}))
36913713
}
36923714

36933715
/// CR 702.62a + CR 702.62b + CR 611.2a + CR 608.2c: Recognize the plural,
@@ -3766,6 +3788,8 @@ pub(super) fn try_parse_exiled_this_way_keyword_grant(
37663788
is_optional: false,
37673789
};
37683790
build_continuous_clause(application, after_head.trim(), ctx)
3791+
.ok()
3792+
.flatten()
37693793
}
37703794

37713795
/// Strip "for each [clause]" suffix from text so that duration extraction can find

0 commit comments

Comments
 (0)