fix TLA orientations in metarules

build.sbt

@@ -20,7 +20,7 @@ import io.github.memo33.metarules.meta._, com.sc4nam.module, module.syntax._
 import Implicits._, Network._, Flags._, RotFlip._, Rule.{CopyTile => %}, group.SymGroup._
 lazy val resolve = module.Main.resolveSafely
 lazy val preimage = module.ReverseResolver.create()
-implicit lazy val context: RuleTransducer.Context = RuleTransducer.Context(resolve, module.RegenerateTileOrientationCache.loadCache(), module.MirrorVariants.preprocessor)
+implicit lazy val context: RuleTransducer.Context = RuleTransducer.Context(module.Main.resolve, module.RegenerateTileOrientationCache.loadCache(), module.MirrorVariants.preprocessor)
 def transduce(rule: Rule[SymTile]): Unit = RuleTransducer(rule)(context).map(_.toRul2String).foreach(println)
 """
 
@@ -71,4 +71,4 @@ libraryDependencies += "tv.cntt" %% "scaposer" % "1.11.1" cross CrossVersion.for
 
 libraryDependencies += "io.github.memo33" %% "scdbpf" % "0.2.0" cross CrossVersion.for3Use2_13
 
-libraryDependencies += "io.github.memo33" %% "metarules" % "0.7.0"
+libraryDependencies += "io.github.memo33" %% "metarules" % "0.7.1"

src/main/scala/module/CompileAllMetarules.scala

@@ -31,7 +31,7 @@ object CompileAllMetarules {
 
   /** Add additional rule generators here.
     */
-  def compileMetarulesOnce(tileOrientationCache: collection.mutable.Map[Int, Set[RotFlip]]): Unit = {
+  def compileMetarulesOnce(tileOrientationCache: RuleTransducer.TileOrientationCache): Unit = {
     LOGGER.info("compiling FlexFly metarule code")
     flexfly.CompileFlexFlyCode.start(tileOrientationCache = tileOrientationCache)
     LOGGER.info("compiling RRW metarule code")

src/main/scala/module/Main.scala

@@ -25,15 +25,17 @@ abstract class AbstractMain {
   lazy val resolveSafely: IdResolver = new PartialFunction[Tile, IdTile] {
     def isDefinedAt(tile: Tile) = resolve.isDefinedAt(tile)
     def apply(tile: Tile) = try resolve.apply(tile) catch {
-      case scala.util.control.NonFatal(e) =>
-        throw new IllegalArgumentException(s"ID resolution failed for tile $tile", e)
+      case e: RuleTransducer.ResolutionFailed => throw e
+      case scala.util.control.NonFatal(e) => throw new RuleTransducer.ResolutionFailed(tile, rule = None, reason = e, frame = None)
     }
   }
 
+  private lazy val shouldIgnoreMirroredOrientations: Set[Int] = MirrorVariants.ignoreMirroredOrientations(resolve)
+
   def main(args: Array[String]): Unit = start()
 
   /** Creates a generator with a new context, runs its start method and outputs the resulting RUL2 code to file. */
-  def start(file: File = file, tileOrientationCache: collection.mutable.Map[Int, Set[RotFlip]] = null): Unit = {
+  def start(file: File = file, tileOrientationCache: RuleTransducer.TileOrientationCache = null): Unit = {
     if (tileOrientationCache == null) {
       RegenerateTileOrientationCache.withCache { cache =>
         start(file, cache)
@@ -54,6 +56,14 @@ abstract class AbstractMain {
           printer.println(rule.toRul2String)
         }
       }
+      // finally remove accumulated orientations that we want to ignore (like accidentally mirrored TLAs)
+      tileOrientationCache.accum.mapValuesInPlace { (id, repr) =>
+        if (shouldIgnoreMirroredOrientations(id)) repr.filterNot(_.flipped)
+        else repr
+      }
+      tileOrientationCache.accum.filterInPlace { (id, repr) =>  // remove from accum if equal to cache (so that regenerateTileOrientationCache stabilizes eventually)
+        !tileOrientationCache.cache.get(id).contains(repr)
+      }
     }
   }
 }

src/main/scala/module/MirrorVariants.scala

@@ -54,23 +54,43 @@ object MirrorVariants {
     case _ => tile
   }
 
-  private val tlaPreprocessor: Rule[SymTile] => Iterator[Rule[SymTile]] = rule => {
-    if (!rule.exists(containsTlaFlags)) {
-      Iterator(rule)
-    } else if (rule.forall(shouldProjectTlaLeftOnly)) {
-      Iterator(rule.map(projectTlaLeft))
-    } else {
-      Iterator(rule.map(projectTlaLeft), rule.map(projectTlaRight))
-    }
+  val preprocessor: Rule[SymTile] => Iterator[Rule[SymTile]] = rule => {
+    val hasMirrorVariant = rule.exists(tile => mirrorVariants.contains(tile))
+    for {
+      rule <- if (hasMirrorVariant) Iterator(  // yield the two projected rules
+                rule.map(tile => mirrorVariants.get(tile).map(_._1).getOrElse(tile)),
+                rule.map(tile => mirrorVariants.get(tile).map(_._2).getOrElse(tile)),
+              )
+              else Iterator(rule)
+      hasTlaFlags = rule.exists(containsTlaFlags)
+      rule <- if (!hasTlaFlags) Iterator(rule)
+              else if (rule.forall(shouldProjectTlaLeftOnly)) Iterator(rule.map(projectTlaLeft))
+              else Iterator(rule.map(projectTlaLeft), rule.map(projectTlaRight))
+      // We duplicate the rule by its R2F1 variant if there are TLAs or mirror
+      // variants involved, as the left/right distinction can make them resolve
+      // to different IDs (and the generators do not necessarily account for
+      // this). For example, this is needed for Tla5/Avenue D×D.
+      rule <- if (hasMirrorVariant || hasTlaFlags) Iterator(rule, rule.map(_ * R2F1)).distinct
+              else Iterator(rule)
+    } yield rule
   }
 
-  val preprocessor: Rule[SymTile] => Iterator[Rule[SymTile]] = rule => {
-    if (!rule.exists(tile => mirrorVariants.contains(tile))) {
-      Iterator(rule)
-    } else {
-      Iterator(  // yield the two projected rules
-        rule.map(tile => mirrorVariants.get(tile).map(_._1).getOrElse(tile)),
-        rule.map(tile => mirrorVariants.get(tile).map(_._2).getOrElse(tile)))
+  /* For plain orthogonal and diagonal tiles of TLA networks, we ignore any
+   * remapping defined in `tileOrientationCache` that leads to mirroring, as
+   * that would amplify the mirroring problems due to the presence of turning
+   * lanes. Not a perfect solution, as occasional mirroring problems will remain.
+   */
+  def ignoreMirroredOrientations(resolve: IdResolver): Set[Int] = {
+    val ids = Set.newBuilder[Int]
+    for {
+      n <- Seq(Tla3, Tla5, Tla7m, Road, Onewayroad)  // TODO consider adding all the networks
+      dir <- Seq(NS, ES, SE)  // these are arbitrary orth/diag directions to allow us find the IDs
+      tile <- Seq(NetworkProperties.projectTlaLeft(n~dir), NetworkProperties.projectTlaRight(n~dir))
+      idTile <- resolve.lift(tile)
+    } {
+      ids += idTile.id
     }
-  }.flatMap(tlaPreprocessor)
+    ids.result()
+  }
+
 }

src/main/scala/module/RegenerateTileOrientationCache.scala

@@ -2,6 +2,7 @@ package com.sc4nam.module
 
 import java.io.File
 import io.github.memo33.metarules.meta.RotFlip
+import syntax.RuleTransducer.TileOrientationCache
 
 /** Manages the tile orientation cache. The cache is necessary to maintain
   * information about non-standard orientations:
@@ -23,19 +24,24 @@ object RegenerateTileOrientationCache {
 
   /** Repeatedly compiles the metarule code until the cache does not get changed anymore.
     */
-  def compileMetarulesUntilStable(tileOrientationCache: collection.mutable.Map[Int, Set[RotFlip]]): Unit = {
+  def compileMetarulesUntilStable(cache: collection.mutable.Map[Int, Set[RotFlip]]): Unit = {
+    val tileOrientationCache = TileOrientationCache(cache = cache, accum = collection.mutable.Map.empty[Int, Set[RotFlip]])
     var j = 0
     var stabilized = false
-    var previous: collection.immutable.Map[Int, Set[RotFlip]] = tileOrientationCache.toMap
-    while (j < 3 && !stabilized) {
-      LOGGER.info(s"> metarules compilation: iteration $j")
+    val maxIter = 5
+    while (j < 5 && !stabilized) {
       j += 1
+      LOGGER.info(s"> metarules compilation: iteration $j")
       CompileAllMetarules.compileMetarulesOnce(tileOrientationCache)
-      val next = tileOrientationCache.toMap
-      if (next == previous) {
+      if (tileOrientationCache.accum.nonEmpty) {
+        tileOrientationCache.cache ++= tileOrientationCache.accum
+        tileOrientationCache.accum.clear()
+        if (j == maxIter) {
+          LOGGER.warning(s"Tile orientation cache could not be regenerated in $maxIter iterations. Increase the number of iterations or check if there is a bug.")
+        }
+      } else {
         stabilized = true
       }
-      previous = next
     }
   }
 
@@ -50,7 +56,7 @@ object RegenerateTileOrientationCache {
     }
   }
 
-  def loadCache(): collection.mutable.Map[Int, Set[RotFlip]] = {
+  def loadCache(): TileOrientationCache = {
     scala.util.Using.resource(new java.util.Scanner(cacheFile, "UTF-8")) { scanner =>
       val cache = collection.mutable.Map.empty[Int, Set[RotFlip]]
       while(scanner.hasNextLine()) {
@@ -62,21 +68,18 @@ object RegenerateTileOrientationCache {
           cache(id) = orientations
         }
       }
-      cache
+      TileOrientationCache(cache = cache, accum = collection.mutable.Map.empty[Int, Set[RotFlip]])
     }
   }
 
   /** Loads the cache and gives a warning at the end if it was changed.
     */
-  def withCache[U](body: collection.mutable.Map[Int, Set[RotFlip]] => U): U = {
-    var previous: collection.immutable.Map[Int, Set[RotFlip]] = null
-    val cache = loadCache()
+  def withCache[U](body: TileOrientationCache => U): U = {
+    val tileOrientationCache = loadCache()
     try {
-      previous = cache.toMap
-      body(cache)
+      body(tileOrientationCache)
     } finally {
-      assert(previous != null)
-      if (previous != cache.toMap) {
+      if (tileOrientationCache.accum.nonEmpty) {
         LOGGER.warning(s"The file ${cacheFile} is outdated. Rebuild it with `sbt regenerateTileOrientationCache` and commit the changes.")
       }
     }

src/test/scala/meta/RuleTransducerSpec.scala

@@ -4,64 +4,62 @@ import scala.collection.immutable.StringOps
 import org.scalatest.wordspec.AnyWordSpec
 import org.scalatest.matchers.should.Matchers
 import com.sc4nam.module, module.syntax._, Implicits._
+import com.sc4nam.module.{NetworkProperties => NP}
 import group._, RotFlip._, SymGroup._, Network._, Flags._
 import RuleTransducer._
 
 class RuleTransducerSpec extends AnyWordSpec with Matchers {
 
   val resolver = new module.RhwResolver orElse new module.NwmResolver orElse new module.MiscResolver
-  val tileOrientationCache = collection.mutable.Map.empty[Int, Set[RotFlip]]
-  val context = RuleTransducer.Context(resolver, tileOrientationCache, module.MirrorVariants.preprocessor)
+  val context = RuleTransducer.Context(resolver, preprocess = module.MirrorVariants.preprocessor)
 
   "preprocessor" should {
     "produce expected number of rules for Tla3" in {
       val orth: Seq[Rule[SymTile]] = context.preprocess( Tla3~WE | (Road ~> Tla3)~WE ).toSeq
       orth should have size (1)
       orth.asInstanceOf[Seq[Rule[Tile]]].exists(_.exists(_.segs.exists(s => s.flags.manifest == Flag.RightSpinBi && s.flags.exists(_ == 2)))) should be (false)
-      createRules(orth.head.map(_.toIdSymTile(resolver)), tileOrientationCache).toSeq should have size (2)
+      createRules(orth.head.map(_.toIdSymTile(resolver)), context.tileOrientationCache.cache, context.tileOrientationCache.accum).toSeq should have size (2)
 
       context.preprocess( Tla3~WE & Road~NS | (Road ~> Tla3)~WE ).toSeq should have size (1)
       val diag = context.preprocess( Tla3~WE & Road~WS | (Road ~> Tla3)~WE ).toSeq
-      diag should have size (2)
+      diag should have size (4)
       for (r <- diag) {
-        createRules(r.map(_.toIdSymTile(resolver)), tileOrientationCache).toSeq should have size (2)
+        createRules(r.map(_.toIdSymTile(resolver)), context.tileOrientationCache.cache, context.tileOrientationCache.accum).toSeq should have size (2)
       }
     }
   }
 
-  def makeTileLeft(t: Tile): Tile = Tile(t.segs map (s => if (!s.network.isTla) s else s.copy(flags = s.flags.spinLeft)))
-  def makeTileRight(t: Tile): Tile = Tile(t.segs map (s => if (!s.network.isTla) s else s.copy(flags = s.flags.spinRight)))
   "Resolver" should {
     "resolve left/right-spinned TLAs correctly" in {
       val tiles = Seq[Tile]( Tla3~WE & Road~ES, Tla3~WE & Ard3~ES, Tla3~WE & Tla3~ES )
       val tile2 = Seq[Tile]( Tla3~WE, Tla3~WE & Road~NS, Tla3~WE & Ard3~NS, Tla3~WE & Tla3~NS )
       for (t <- tiles) {
-        resolver(makeTileLeft(t)) should not be resolver(makeTileRight(t))
+        resolver(NP.projectTlaLeft(t)) should not be resolver(NP.projectTlaRight(t))
       }
       for (t <- tile2) {
-        resolver(makeTileLeft(t)) should be (resolver(makeTileRight(t)))
+        resolver(NP.projectTlaLeft(t)) should be (resolver(NP.projectTlaRight(t)))
       }
     }
     "handle flipped left/right-spinned TLAs correctly" in {
       val (t1, t2) = ( Tla3~WE & Road~ES, Tla3~WE & Road~WS )
-      resolver(makeTileLeft(t1)).id should not be (resolver(makeTileLeft(t2)).id)
+      resolver(NP.projectTlaLeft(t1)).id should not be (resolver(NP.projectTlaLeft(t2)).id)
       for ((t, i) <- Seq(t1, t2).zipWithIndex) {
-        makeTileLeft(t).toIdSymTile(resolver).repr.filter(_.flipped ^ (i!=0)) should be (Symbol("empty"))
-        makeTileRight(t).toIdSymTile(resolver).repr.filter(!_.flipped ^ (i!=0)) should be (Symbol("empty"))
+        NP.projectTlaLeft(t).toIdSymTile(resolver).repr.filter(_.flipped ^ (i!=0)) should be (Symbol("empty"))
+        NP.projectTlaRight(t).toIdSymTile(resolver).repr.filter(!_.flipped ^ (i!=0)) should be (Symbol("empty"))
       }
     }
     "find RHS for TLA" in {
-      val rule = (Tla3~WE | (Road ~> Tla3)~(2,0,11,0)) map makeTileLeft map (_.toIdSymTile(resolver))
+      val rule = (Tla3~WE | (Road ~> Tla3)~(2,0,11,0)) map NP.projectTlaLeft map (_.toIdSymTile(resolver))
       possibleMapOrientation(Set(R0F0, R1F0), R3F0/R2F1, Quotient.Dih4, R1F1/R2F1) should not be (Symbol("empty"))
-      createRules(rule, tileOrientationCache)
+      createRules(rule, context.tileOrientationCache.cache, context.tileOrientationCache.accum)
     }
     "resolve diagonal TLA intersections" in {
-      val t1 = makeTileLeft(Tla3~ES & Road~WS)
-      val t2 = makeTileRight(Tla3~ES & Road~WS)
-      val t3 = makeTileLeft(Tla3~WS & Road~ES)
+      val t1 = NP.projectTlaLeft(Tla3~ES & Road~WS)
+      val t2 = NP.projectTlaRight(Tla3~ES & Road~WS)
+      val t3 = NP.projectTlaLeft(Tla3~WS & Road~ES)
       resolver(t3) * R0F1 should be (resolver(t2))
       resolver(t1).id should not be (resolver(t2).id)
-      resolver(makeTileLeft(Tla3~ES & Tla3~WS)) should be (resolver(makeTileRight(Tla3~ES & Tla3~WS)))
+      resolver(NP.projectTlaLeft(Tla3~ES & Tla3~WS)) should be (resolver(NP.projectTlaRight(Tla3~ES & Tla3~WS)))
     }
   }
 }

src/test/scala/module/MirrorVariantsSpec.scala

@@ -56,29 +56,29 @@ class MirrorVariantsSpec extends AnyWordSpec with Matchers {
 
     "produce expected results for TLA network crossings" in {
       implicit val context = RuleTransducer.Context(resolve, preprocess = MirrorVariants.preprocessor)
-      RuleTransducer(Tla3~WE | (Road~>Tla3)~WE & Rail~ES).toSeq shouldBe Seq(
+      RuleTransducer(Tla3~WE | (Road~>Tla3)~WE & Rail~ES).toSeq should contain theSameElementsAs Seq(
         Rule(0x51000000,1,0, 0x03010200,1,0, 0x51000000,1,0, 0x51005500,3,0),
         Rule(0x51000000,3,0, 0x03010200,1,0, 0x51000000,3,0, 0x51005500,3,0),
         Rule(0x51000000,3,0, 0x03020500,3,1, 0x51000000,3,0, 0x51005500,1,1),
         Rule(0x51000000,1,0, 0x03020500,3,1, 0x51000000,1,0, 0x51005500,1,1))
-      RuleTransducer(Tla5~WE | (Road~>Tla5)~WE & Rail~ES).toSeq shouldBe Seq(
+      RuleTransducer(Tla5~WE | (Road~>Tla5)~WE & Rail~ES).toSeq should contain theSameElementsAs Seq(
         Rule(0x51100000,3,0, 0x03010200,1,0, 0x51100000,3,0, 0x51105500,3,0),
         Rule(0x51100000,1,0, 0x03020500,3,1, 0x51100000,1,0, 0x51105500,1,1))
-      RuleTransducer(Tla5~WE | (Road~>Tla5)~WE & Road~ES).toSeq shouldBe Seq(
+      RuleTransducer(Tla5~WE | (Road~>Tla5)~WE & Road~ES).toSeq should contain theSameElementsAs Seq(
         Rule(0x51100000,3,0, 0x00003900,1,0, 0x51100000,3,0, 0x51105100,3,0),
         Rule(0x51100000,1,0, 0x00003900,3,1, 0x51100000,1,0, 0x71105100,1,1))  // 0x71... variant
-      RuleTransducer(Tla3~WE | (Road~>Tla3)~WE & Lightrail~ES).toSeq shouldBe Seq(
+      RuleTransducer(Tla3~WE | (Road~>Tla3)~WE & Lightrail~ES).toSeq should contain theSameElementsAs Seq(
         Rule(0x51000000,1,0, 0x08DD1600,1,1, 0x51000000,1,0, 0x51005600,3,0),
         Rule(0x51000000,3,0, 0x08DD1600,1,1, 0x51000000,3,0, 0x51005600,3,0),
         Rule(0x51000000,3,0, 0x08DD1600,3,0, 0x51000000,3,0, 0x51005600,1,1),
         Rule(0x51000000,1,0, 0x08DD1600,3,0, 0x51000000,1,0, 0x51005600,1,1))
-      RuleTransducer(Tla5~EW | (Avenue~>Tla5)~EW & Avenue~NS).toSet shouldBe Set(
+      RuleTransducer(Tla5~EW | (Avenue~>Tla5)~EW & Avenue~NS).toSeq should contain theSameElementsAs Seq(
         Rule(0x51100000,3,0, 0x04009000,0,0, 0x51100000,3,0, 0x71101300,2,1),
         Rule(0x51100000,1,0, 0x04009000,1,0, 0x51100000,1,0, 0x51101300,0,0))
-      RuleTransducer(Tla5~EW & Avenue~NS | (Avenue~>Tla5)~EW & Avenue~SN).toSet shouldBe Set(
+      RuleTransducer(Tla5~EW & Avenue~NS | (Avenue~>Tla5)~EW & Avenue~SN).toSeq should contain theSameElementsAs Seq(
         Rule(0x71101300,2,1, 0x04009000,3,0, 0x71101300,2,1, 0x51101300,2,0),
         Rule(0x51101300,0,0, 0x04009000,2,0, 0x51101300,0,0, 0x71101300,0,1))
-      RuleTransducer(Tla5~EW & Avenue~SN | (Avenue~>Tla5)~EW).toSet shouldBe Set(
+      RuleTransducer(Tla5~EW & Avenue~SN | (Avenue~>Tla5)~EW).toSeq should contain theSameElementsAs Seq(
         Rule(0x51101300,2,0, 0x04006100,3,0, 0x51101300,2,0, 0x51100000,3,0),
         Rule(0x71101300,0,1, 0x04006100,1,0, 0x71101300,0,1, 0x51100000,1,0))
     }