don't promote recall from alts to core without matching remember

Source/Data/Requirement.cs

@@ -65,6 +65,18 @@ public bool IsMeasured
             }
         }
 
+        /// <summary>
+        /// Gets whether or not the requirement references a remembered value.
+        /// </summary>
+        public bool HasRecall
+        {
+            get
+            {
+                return Left.Type == FieldType.Recall || Right.Type == FieldType.Recall;
+            }
+        }
+
+
         /// <summary>
         /// Gets or sets the requirement operator.
         /// </summary>

Source/Data/RequirementEx.cs

@@ -83,6 +83,20 @@ public bool HasHitCount
             }
         }
 
+        public bool HasRecall
+        {
+            get
+            {
+                foreach (var requirement in Requirements)
+                {
+                    if (requirement.HasRecall)
+                        return true;
+                }
+
+                return false;
+            }
+        }
+
         public override string ToString()
         {
             if (Requirements.Count == 1)

Source/Parser/Internal/RequirementsOptimizer.cs

@@ -1,6 +1,7 @@
 using RATools.Data;
 using RATools.Parser.Expressions;
 using RATools.Parser.Functions;
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
@@ -569,6 +570,34 @@ private static void PromoteCommonAltsToCore(List<List<RequirementEx>> groups, Li
                 }
             }
 
+            // Remember and Recall cannot be separated. If any Recall items are being promoted, make sure
+            // the Remember and all other Recalls are also being promoted.
+            Predicate<RequirementEx> hasRememberOrRecall = r => r.Requirements.Any(r2 => r2.Type == RequirementType.Remember || r2.HasRecall);
+            if (requirementsFoundInAll.Any(r => hasRememberOrRecall(r)))
+            {
+                for (int i = 1; i < groups.Count; i++)
+                {
+                    bool allPromoted = true;
+                    foreach (var requirementEx in groups[i])
+                    {
+                        if (hasRememberOrRecall(requirementEx))
+                        {
+                            if (!requirementsFoundInAll.Contains(requirementEx))
+                            {
+                                allPromoted = false;
+                                break;
+                            }
+                        }
+                    }
+
+                    if (!allPromoted)
+                    {
+                        requirementsFoundInAll.RemoveAll(r => hasRememberOrRecall(r));
+                        break;
+                    }
+                }
+            }
+
             // remove the redundant requirements from each alt group
             if (requirementsFoundInAll.Any())
             {
@@ -1395,7 +1424,7 @@ private static RequirementEx GetRemembered(List<Requirement> requirements)
                         break;
 
                     j--;
-                    if (requirements[j].Left.Type == FieldType.Recall || requirements[j].Right.Type == FieldType.Recall)
+                    if (requirements[j].HasRecall)
                         hasRecall = true;
                 }
 
@@ -1458,11 +1487,8 @@ internal static int FindResetNextIfStart(IList<Requirement> requirements, int re
                         // these have higher precedence than ResetNextIf, drag them with
                         i--;
 
-                        if (requirements[i].Left.Type == FieldType.Recall ||
-                            requirements[i].Right.Type == FieldType.Recall)
-                        {
+                        if (requirements[i].HasRecall)
                             hasRecall = true;
-                        }
 
                         continue;
 

Source/Parser/ScriptBuilderContext.cs

@@ -1,6 +1,7 @@
 using RATools.Data;
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Text;
 
@@ -52,6 +53,7 @@ public ScriptBuilderContext Clone()
         private Requirement _lastAndNext;
         private int _remainingWidth;
 
+        [DebuggerDisplay("{Requirement}")]
         class MemoryAccessorAliasChain
         {
             public MemoryAccessorAlias Alias { get; set; }
@@ -154,7 +156,7 @@ public void AppendRequirements(StringBuilder builder, IEnumerable<Requirement> r
                     if (group.Requirements.Any(r => r.Type == RequirementType.Remember))
                         break;
 
-                    if (group.Requirements.Any(r => r.Left.Type == FieldType.Recall || r.Right.Type == FieldType.Recall))
+                    if (group.HasRecall)
                     {
                         // found a Recall without a Remember, it will use the last non-pause Remember
                         hasNonPauseRecallBeforeLastPauseRemember = true;
@@ -253,7 +255,6 @@ private void AppendRequirementEx(StringBuilder builder, RequirementEx requiremen
                                 Next = null,
                                 Requirement = requirement,
                             };
-                            continue;
                         }
                         else if (requirement.Left.IsMemoryReference)
                         {
@@ -850,7 +851,17 @@ private void AppendFieldAlias(StringBuilder builder, Field field, MemoryAccessor
                     builder.Append('~');
                     break;
                 case FieldType.Recall:
-                    builder.Append(_remember);
+                    if (parentRequirement.Operator != RequirementOperator.None)
+                    {
+                        builder.Append('(');
+                        builder.Append(_remember);
+                        builder.Append(')');
+                        AppendFieldModifier(builder, parentRequirement);
+                    }
+                    else
+                    {
+                        builder.Append(_remember);
+                    }
                     return;
             }
 

Tests/Parser/Internal/RequirementsOptimizerTests.cs

@@ -225,6 +225,21 @@ public void TestOptimizeNormalizeResetIfsAndPauseIfs(string input, string expect
                   "once(byte(0x001234) == 1) && ((once(byte(0x002345) == 2) && unless(byte(0x002345) == 1)))")]
         [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
                   "once(byte(0x001234) == 1) && disable_when(byte(0x001234) == 1) && (never(byte(0x002345) == 1))")]
+        [TestCase("1 == 1 && (byte((word(0x001234) & 0x3FF) * 4 + 8) == 6 || byte((word(0x001234) & 0x3FF) * 4 + 16) == 8)", // common remember should not be promoted to core without all recall references.
+                  "(byte((word(0x001234) & 0x000003FF) * 4 + 0x08) == 6) || (byte((word(0x001234) & 0x000003FF) * 4 + 0x10) == 8)")]
+        [TestCase("1 == 1 && (byte((word(0x001234) & 0x3FF) * 4 + 8) == 6 || byte((word(0x002345) & 0x3FF) * 4 + 8) == 6)", // common remember should not be promoted to core without all recall references.
+                  "(byte((word(0x001234) & 0x000003FF) * 4 + 0x08) == 6) || (byte((word(0x002345) & 0x000003FF) * 4 + 0x08) == 6)")]
+        [TestCase("1 == 1 && ((byte((word(0x001234) & 0x3FF) * 4 + 8) == 6 && prev(byte((word(0x001234) & 0x3FF) * 4 + 8)) == 5) || " +
+                             "(byte((word(0x002345) & 0x3FF) * 4 + 8) == 6 && prev(byte((word(0x002345) & 0x3FF) * 4 + 8)) == 5))", // common remember should not be promoted to core without all recall references.
+                  "(byte((word(0x001234) & 0x000003FF) * 4 + 0x08) == 6 && prev(byte((word(0x001234) & 0x000003FF) * 4 + 0x08)) == 5) || " +
+                  "(byte((word(0x002345) & 0x000003FF) * 4 + 0x08) == 6 && prev(byte((word(0x002345) & 0x000003FF) * 4 + 0x08)) == 5)")]
+        [TestCase("1 == 1 && ((byte(remembered((word(0x001234) & 0x3FF) * 4) + 8) == 6 && prev(byte(remembered((word(0x001234) & 0x3FF) * 4) + 8)) == 5) || " +
+                             "(byte(remembered((word(0x002345) & 0x3FF) * 4) + 8) == 6 && prev(byte(remembered((word(0x002345) & 0x3FF) * 4) + 8)) == 5))", // common remember should not be promoted to core without all recall references.
+                  "(byte((word(0x001234) & 0x000003FF) * 4 + 0x08) == 6 && prev(byte((word(0x001234) & 0x000003FF) * 4 + 0x08)) == 5) || " +
+                  "(byte((word(0x002345) & 0x000003FF) * 4 + 0x08) == 6 && prev(byte((word(0x002345) & 0x000003FF) * 4 + 0x08)) == 5)")]
+        [TestCase("1 == 1 && ((byte(remembered((word(0x001234) & 0x3FF) * 4) + 8) == 6 && prev(byte(remembered((word(0x001234) & 0x3FF) * 4) + 8)) == 5) || " +
+                             "(byte(remembered((word(0x001234) & 0x3FF) * 4) + 8) == 6 && prev(byte(remembered((word(0x001234) & 0x3FF) * 4) + 8)) == 5))", // common remember can be promoted to core with all recall references.
+                  "byte((word(0x001234) & 0x000003FF) * 4 + 0x08) == 6 && prev(byte((word(0x001234) & 0x000003FF) * 4 + 0x08)) == 5")]
         public void TestOptimizePromoteCommonAltsToCore(string input, string expected)
         {
             var achievement = CreateAchievement(input);

Tests/Parser/Internal/ScriptBuilderContextTests.cs

@@ -69,6 +69,7 @@ class ScriptBuilderContextTests
                   "((once(word(0x000002) == 2) && word(0x000003) == 3) || word(0x000004) == 4) && never(word(0x000001) == 1)")]
         [TestCase("I:0xX001234_A:0xH000001/0xH000001_0=1",
                   "(byte(dword(0x001234) + 0x01) / byte(dword(0x001234) + 0x01)) == 1")]
+        [TestCase("K:0x 001234&1023_I:{recall}*4_0xH000008=6", "byte((word(0x001234) & 0x3FF) * 4 + 0x08) == 6")]
         public void TestAppendRequirements(string input, string expected)
         {
             var trigger = Trigger.Deserialize(input);