Skip to content

Commit 7e1c3eb

Browse files
JamirasJamiras
authored
don't eliminate ResetNextIf attached to PauseLock when it matches an outer ResetIf (#556)
1 parent d0ab249 commit 7e1c3eb

File tree

4 files changed

+70
-21
lines changed

4 files changed

+70
-21
lines changed

‎Source/Parser/Internal/RequirementsOptimizer.cs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,14 +1253,24 @@ private static void NormalizeResetNextIfs(List<List<RequirementEx>> groups)
12531253
for (int k = j; k <= i; k++)
12541254
subclause.Requirements.Add(requirementEx.Requirements[k]);
12551255

1256-
Debug.Assert(j < requirementEx.Requirements.Count - 1);
1257-
subclause.Requirements.Last().Type = RequirementType.ResetIf;
1258-
if (group.Any(reqEx => reqEx == subclause))
1256+
if (lastRequirement.Type == RequirementType.PauseIf)
12591257
{
1260-
requirementEx.Requirements.RemoveRange(j, i - j + 1);
1261-
continue;
1258+
// ResetNextIf used to clear hits on a Pause cannot be extracted
1259+
resetNextIsForPause = true;
1260+
}
1261+
else
1262+
{
1263+
// ResetNextIf that exactly matches a ResetIf outside the clause
1264+
// is covered by the ResetIf outside the clause.
1265+
Debug.Assert(j < requirementEx.Requirements.Count - 1);
1266+
subclause.Requirements.Last().Type = RequirementType.ResetIf;
1267+
if (group.Any(reqEx => reqEx == subclause))
1268+
{
1269+
requirementEx.Requirements.RemoveRange(j, i - j + 1);
1270+
continue;
1271+
}
1272+
subclause.Requirements.Last().Type = RequirementType.ResetNextIf;
12621273
}
1263-
subclause.Requirements.Last().Type = RequirementType.ResetNextIf;
12641274

12651275
hasResetNextIf = true;
12661276

@@ -1294,8 +1304,6 @@ private static void NormalizeResetNextIfs(List<List<RequirementEx>> groups)
12941304
}
12951305

12961306
resetNextIfClauses.Add(requirementEx);
1297-
1298-
resetNextIsForPause |= (lastRequirement.Type == RequirementType.PauseIf);
12991307
}
13001308
}
13011309
}

‎Source/Parser/ScriptBuilderContext.cs

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ public void AppendRequirement(StringBuilder builder, Requirement requirement)
475475
break;
476476

477477
case RequirementType.PauseIf:
478-
if (_resetNextIf != null)
478+
if (_resetNextIf != null || requirement.HitCount != 0)
479479
{
480480
var nestedContext = CreatedNestedContext();
481481
nestedContext._resetNextIf = null;
@@ -489,12 +489,17 @@ public void AppendRequirement(StringBuilder builder, Requirement requirement)
489489
}
490490

491491
builder.Append("disable_when(");
492-
builder.Append(comparison.ToString());
493-
builder.Append(", until=");
494-
builder.Append(_resetNextIf);
495-
builder.Append(')');
492+
builder.Append(comparison);
493+
494+
if (_resetNextIf != null)
495+
{
496+
builder.Append(", until=");
497+
builder.Append(_resetNextIf);
498+
499+
_resetNextIf.Clear();
500+
}
496501

497-
_resetNextIf.Clear();
502+
builder.Append(')');
498503
}
499504
else
500505
{
@@ -884,11 +889,20 @@ private void AppendTally(StringBuilder builder, IEnumerable<Requirement> require
884889
var repeated = new StringBuilder();
885890
nestedContext.AppendRequirementEx(repeated, requirementEx);
886891

892+
string suffix = "";
887893
var repeatedString = repeated.ToString();
894+
var index = repeated.Length;
888895
var repeatedIndex = repeatedString.IndexOf("repeated(");
889-
var onceIndex = repeatedString.IndexOf("once(");
890-
var index = 0;
891-
if (repeatedIndex >= 0 && (onceIndex == -1 || repeatedIndex < onceIndex))
896+
if (repeatedIndex != -1)
897+
index = repeatedIndex;
898+
var onceIndex = repeatedString.IndexOf("once(", 0, index);
899+
if (onceIndex != -1)
900+
index = onceIndex;
901+
var disableWhenIndex = repeatedString.IndexOf("disable_when(", 0, index);
902+
if (disableWhenIndex != -1)
903+
index = disableWhenIndex;
904+
905+
if (index == repeatedIndex)
892906
{
893907
// replace the "repeated(" with "tally("
894908
builder.Append(repeatedString, 0, repeatedIndex);
@@ -901,14 +915,36 @@ private void AppendTally(StringBuilder builder, IEnumerable<Requirement> require
901915

902916
index = repeatedIndex + 2;
903917
}
904-
else if (onceIndex >= 0)
918+
else if (index == onceIndex)
905919
{
906920
// replace the "once(" with "tally(1, "
907921
builder.Append(repeatedString, 0, onceIndex);
908922
builder.Append("tally(1, ");
909923

910924
index = onceIndex + 5;
911925
}
926+
else if (index == disableWhenIndex)
927+
{
928+
builder.Append(repeatedString, 0, disableWhenIndex);
929+
index = disableWhenIndex + 13;
930+
931+
if (index == repeatedIndex)
932+
{
933+
// replace the "disable_when(repeated(N, " with "disable_when(tally(N, "
934+
builder.Append("disable_when(tally(");
935+
index += 9;
936+
var comma = repeatedString.IndexOf(',', index);
937+
builder.Append(repeatedString, index, comma - index);
938+
index = comma + 2; // assume ", "
939+
builder.Append(", ");
940+
}
941+
else
942+
{
943+
// replace the "disable_when(" with "unless(tally(1, "
944+
builder.Append("disable_when(tally(1, ");
945+
suffix = ")";
946+
}
947+
}
912948

913949
// append the AddHits subclauses
914950
if (WrapWidth != Int32.MaxValue)
@@ -983,6 +1019,8 @@ private void AppendTally(StringBuilder builder, IEnumerable<Requirement> require
9831019
}
9841020

9851021
builder.Append(remaining);
1022+
builder.Append(suffix);
1023+
9861024
_remainingWidth = WrapWidth - Indent - remaining.Length;
9871025
}
9881026
}

‎Tests/Parser/Internal/RequirementsOptimizerTests.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ public void TestOptimizeNormalizeComparisons(string input, string expected)
167167
[TestCase("unless(byte(0x001234) < 5)", "byte(0x001234) >= 5")]
168168
[TestCase("unless(byte(0x001234) != 1 && byte(0x002345) == 2)", "byte(0x001234) == 1 || byte(0x002345) != 2")] // AndNext becomes OrNext, both operators inverted
169169
[TestCase("unless(byte(0x001234) == 5) && byte(0x002345) == 1", "byte(0x001234) != 5 && byte(0x002345) == 1")] // unless without HitCount should be inverted to a requirement
170-
[TestCase("unless(byte(0x001234) != 1) && unless(once(byte(0x002345) == 1))", "unless(byte(0x001234) != 1) && unless(once(byte(0x002345) == 1))")] // PauseLock is affected by Pause, so other Pause won't be inverted
170+
[TestCase("unless(byte(0x001234) != 1) && unless(once(byte(0x002345) == 1))", // PauseLock is affected by Pause, so other Pause won't be inverted
171+
"unless(byte(0x001234) != 1) && disable_when(byte(0x002345) == 1)")]
171172
[TestCase("byte(0x001234) == 5 && never(byte(0x001234) != 5)", "byte(0x001234) == 5")] // common pattern in older achievements to fix HitCount at 0, the ResetIf is functionally redundant
172173
[TestCase("(byte(0x002345) == 5 && never(byte(0x001234) == 6)) || (byte(0x002345) == 6 && never(byte(0x001235) == 3))",
173174
"(byte(0x002345) == 5 && byte(0x001234) != 6) || (byte(0x002345) == 6 && byte(0x001235) != 3)")] // same logic applies to alt groups
@@ -181,6 +182,8 @@ public void TestOptimizeNormalizeComparisons(string input, string expected)
181182
"trigger_when(repeated(3, byte(0x001234) == 1 && never(byte(0x002345) == 2))) && byte(0x003456) == 3")]
182183
[TestCase("byte(0x001234) == 1 || (unless(byte(0x002345) == 1) && never(always_true()))", // ResetIf guarded by PauseIf should be kept
183184
"byte(0x001234) == 1 || (unless(byte(0x002345) == 1) && never(always_true()))")]
185+
[TestCase("once(byte(0x001234) == 1) && disable_when(byte(0x002345) == 2, until=byte(0x003456) == 1) && never(byte(0x003456) == 1)", // inner ResetNextIf must be kept to reset PauseIf even if handled by outer ResetIf
186+
"once(byte(0x001234) == 1) && disable_when(byte(0x002345) == 2, until=byte(0x003456) == 1) && never(byte(0x003456) == 1)")]
184187
public void TestOptimizeNormalizeResetIfsAndPauseIfs(string input, string expected)
185188
{
186189
var achievement = CreateAchievement(input);
@@ -219,7 +222,7 @@ public void TestOptimizeNormalizeResetIfsAndPauseIfs(string input, string expect
219222
[TestCase("once(byte(0x001234) == 1) && (always_false() || once(byte(0x002345) == 2) && unless(byte(0x002345) == 1))", // always_false group is discarded, unless is not promoted because of hit target
220223
"once(byte(0x001234) == 1) && ((once(byte(0x002345) == 2) && unless(byte(0x002345) == 1)))")]
221224
[TestCase("once(byte(0x001234) == 1) && unless(once(byte(0x001234) == 1)) && (always_false() || never(byte(0x002345) == 1))", // never should not be promoted to core containing unless
222-
"once(byte(0x001234) == 1) && unless(once(byte(0x001234) == 1)) && (never(byte(0x002345) == 1))")]
225+
"once(byte(0x001234) == 1) && disable_when(byte(0x001234) == 1) && (never(byte(0x002345) == 1))")]
223226
public void TestOptimizePromoteCommonAltsToCore(string input, string expected)
224227
{
225228
var achievement = CreateAchievement(input);

‎Tests/Parser/Internal/ScriptBuilderContextTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class ScriptBuilderContextTests
4949
[TestCase("A:2_C:d0xL001234<=0xL001234_N:0xU001234=d0xU001234_T:d0xH001234<=0xH001234.70.",
5050
"trigger_when(tally(70, (2 + prev(low4(0x001234))) <= low4(0x001234), high4(0x001234) == prev(high4(0x001234)) && prev(byte(0x001234)) <= byte(0x001234)))")]
5151
[TestCase("C:0xH001234=1.1._P:1=1.1.",
52-
"unless(tally(1, once(byte(0x001234) == 1), always_true()))")]
52+
"disable_when(tally(1, once(byte(0x001234) == 1), always_true()))")]
5353
[TestCase("C:0xH001234=1.1._D:0xH002345=1.1._R:0=1.1.",
5454
"never(tally(1, once(byte(0x001234) == 1), deduct(once(byte(0x002345) == 1))))")]
5555
[TestCase("R:0xH001234<5_R:0xH001234>8_N:0xH002345=1_0xH003456=2.10.",

0 commit comments

Comments
 (0)