[DLL] Remove redundant RUL2 adjacencies

[memo33]

For integration with the DLL-based RUL2 engine, this PR aims to remove a lot of RUL2 code made redundant by the adjacency handling of the DLL.

DLL-based adjacencies in a nutshell

Whenever you have two adjacent tiles A | B, the engine looks for an orthogonal surrogate tile S and matching RUL2 rules A | S ⇒ A' | S' as well as S' | B ⇒ S" | B' that make an override carry over from A to S to B. It then applies the result to the original tiles A | B directly, effectively applying a "virtual" rule A | B ⇒ A' | B'.

This means you only need to write RUL2 code that connects to pure orthogonals (in and out), but no adjacencies.

Diagonals work similarly, except they use two diagonal surrogate tiles. The effect is the same in that you only write RUL2 code connecting to pure diagonals (in and out).

(The orthogonal and diagonal surrogate tiles are chosen from a hard-coded list of IIDs.)

Goals of this PR

• Write a script to identify redundant RUL2 code (i.e. covered by DLL + other RUL2 code).

• Remove adjacencies from metarules:

  • FlexFlyRuleGenerator
  • OST generator
  • RhwRuleGenerator
  • RealRailwayRuleGenerator (contained no adjacencies in the first place)
  • SamRuleGenerator (covered by meta-sam branch)

  @TarkusSC4 @jflann I'm not that familiar with the state of the last two generator files. Perhaps you could take a look and remove adjacencies?

• Run removal script and regenerate metarule-based RUL2 code (needs to rerun after previous step is complete).

• Sight the remaining RUL2 code in the repository and remove redundant parts manually:

  • Some files seem to consist of almost entirely adjacencies only, for example:

    • Controller/RUL2/07_RHW/Sec7b_BaseNetwork
    • Controller/RUL2/08_NWM/

    Perhaps, someone more familiar with those files has a good idea about what could be removed easily? @TarkusSC4
    Partially scripted, see remove-rhw-adjacencies.py and remove-nwm-adjacencies.py.

  • Some of the handwritten adjacency code seems to have some bugs in it. The script cannot identify these, so it tends to leave behind batches of buggy code. It would make sense to manually check whether some of the remaining RUL2 code should be (a) deleted or (b) fixed.
    Partially scripted, see SegmentOrientationChecker and MissingInstanceChecker.

Help wanted! Especially the manual sighting is a huge task. It would be effectively open-ended, depending on how much of a clean-up we want this to be. The manual steps could even be skipped entirely without any cleanup (which shouldn't be less stable than the NAM 48 controller currently is).

[memo33]

[jflann]

Nice job so far with all this.

I've largely re-written the SAM rule generator on the meta-sam branch with the NAM DLL in mind. It shouldn't produce any adjacencies and should be very close to fully functional and bug-free. I'm not sure the best way to integrate that work into this PR, but I think it's basically ready.

I'll still need to look over the RRW rule generator but I don't think there are many adjacency-related rules there to begin with.

[memo33]

Ah, thanks. For the meta-sam branch, would it make sense to use that code without the DLL as well? In that case, it could be merged into staging. Otherwise, we can merge it here eventually. It probably doesn't hurt to keep the branches separate for now till they're basically ready.

[TarkusSC4]

Regarding 07_RHW/Sec7b_BaseNetwork and 08_NWM, I can confirm that most of the files contained in there are massive collections of adjacencies, and huge chunks can be pretty safely removed. There's likely only some select oddball things in those files that still require specialized code that isn't covered by the RhwRuleGenerator and the DLL's automatic adjacency handling, and most of them are going to be in the respective 00_Orthogonal and 02_Diagonal files. It'll require a bit of testing, but I have a fairly good idea of where to start with it.

[jflann]

Ah, thanks. For the meta-sam branch, would it make sense to use that code without the DLL as well?

No, I've made a lot of changes to the street network in light of the new diagonal functionality, so it only makes sense to merge for a post-dll version. I agree, we can just keep them separate.

[jflann]

RealRailwayRuleGenerator doesn't produce any adjacencies, so should be good to go.

[memo33]

I've updated the script so that it loads files in a deterministic order. Now the load order is the same as the one used by the controller compiler and the game. I've also restructured the Scala code a little bit. See the individual commits.

Finally, I reran the script (twice) from the original state of the RUL2 folder in order to make sure to discard any RUL2 changes made by previous versions of the script – as can be seen in the git history (branches/merges). I've discarded previous runs of the script using merge --strategy=ours.

The following diff compares yesterday's state of the RUL2 folder with the current one. In other words, this shows exactly the effect of switching to a deterministic load order. This way, even more redundant adjacencies were found:

Showing 484 changed files with 23,817 additions and 29,262 deletions.

328bb2e...a6c9d12

[memo33]

I've rebased the branch (forced push) to factor out common functionality shared with #494.

[memo33]

Rebased on top of staging.

Interestingly, the removal of redundant adjacencies simultaneously eliminates a lot of conflicting overrides, not only in lines that have been removed, but also in lines that still exist. From the last commit:

Found 39173 conflicting RUL2 overrides (removed 37303, added 81).

[jflann]

I have started to look at some of the leftover adjacency files, mainly the RHW branch adjacencies. At least in the first few I've reviewed, the leftover rules (< 100, typ.) seem to fall in a few predictable categories:

• Duplicate of foundational rules that exist in the FLEXRamp files
• Adjacency for pieces which don't exist (either typos or not created yet, hard to know)
• Rules which don't make sense (e.g., rules which require do a double switch)
• Stuff involving RRW which isn't implemented, but if it were, would make these rules redundant.
• OST code which overrides L1 versions of the RHW transition (which I'm guessing is a mitigation for some auto L1 behavior which may not be needed any longer.
• A few good alt-GLR rules which could probably be made redundant by addressing the problem within the GLR codebase or the RHW generator.

Considering that, I think the best course of action for each of these files would be to verify that the foundational rules exist in FLEXRamp, then load up the remainder in the RUL Viewer to verify nothing else important is being deleted, then delete the entire file. If that's agreeable, I will start making changes.

[memo33]

Considering that, I think the best course of action for each of these files would be to verify that the foundational rules exist in FLEXRamp, then load up the remainder in the RUL Viewer to verify nothing else important is being deleted, then delete the entire file. If that's agreeable, I will start making changes.

I mostly agree. Though, I'm a bit worried by the amount of files, which can quickly get exhausting if you open them all in the RUL Viewer. If you see any patterns that allow eliminating larger chunks without having to manually go through the code, that could be helpful. For example, after adding rules for the alt-GLRs, rerunning the redundant-adjacencies script could eliminate some rules, although I guess that will cover just a small amount.

[jflann]

If you see any patterns that allow eliminating larger chunks without having to manually go through the code, that could be helpful.

Something that would help would be to delete parts of the code for height levels which don't exist. For example, RHW-2 only exists at L0-L2, but there are blocks of code in the ramp files for L3 and L4. This seems to be repeated across other networks as well. These are conveniently high-level subsections within the ramp adjacency files so they are easy to identify and delete. They seem to all begin and end with clearly identifiable block comments:

;-----------------------
;----L4 MIS (0x5742)----
;-----------------------

...

;------------------
;----END L4 MIS----
;------------------

[memo33]

Do you have a link to an example? I've done a quick search, but couldn't really find such blocks for networks other than MIS, RHW-4 and RHW-6S, which do support those height levels.

[jflann]

Do you have a link to an example? I've done a quick search, but couldn't really find such blocks for networks other than MIS, RHW-4 and RHW-6S, which do support those height levels.

Here are some examples. Maybe not as widespread as I was anticipating.

[memo33]

Ah, I see. I didn't know how the RHW ramp adjacencies were structured, but I think I get it now. What you describe is the following, right?

• remove all the blocks for L3 MIS, L4 MIS, L3 RHW-4, L4 RHW-4, L3 RHW-6S, L4 RHW-6S using the format quoted above
• from the files Controller/RUL2/07_RHW/Sec7b_BaseNetwork/Ramp* – excluding the following files, as these contain ramps that may support those height levels:
  • Ramp_{A,B,C,D,E}*
  • Ramp_4_*
  • Ramp_6S_*
  • Ramp_MIS_*

Does that sound right? What about non-ramp files?

Another idea would be to remove all rules that use IDs for which no path file exists. (Some intermediate dummy tiles don't have a texture or model, but a path file should always exist to circumvent lag while dragging.)

[jflann]

Yes, I think you've got it right. I think only the branch code is organized into network-specific files. The other ramp files have multiple networks in them.

Originally, I was thinking this would be pretty helpful to reduce the amount of code to look at, but I think this might be a distraction and instead we should try to focus on figuring out how the code is structured with the goal of removing entire files. This more than anything will really help to make things less overwhelming.

I'm wondering if maybe it would be easier to verify that the correct basic overrides exist in the FLEXRamp code and then just delete the corresponding ramp adjacency files? We know that these files contain adjacency involving top, bottom, or branch ramp tiles, so we just need to check that the special case (adjacency to base ortho or base diag) is intact elsewhere. In fact, this is where a script to check for identical rules would be helpful. Then, we could manipulate the loading order to ensure FLEXRamp loads before the ramp adjacency files, and tag/delete the duplicate lines in the adjacency. That would give us the confidence to just delete the whole file.

I also like the idea of removing rules for tiles that lack path files, but we'd have to be careful because I think it's fairly common for there to be "future-proof" rules created. Stuff like that should hopefully be commented out, but you never know.

[memo33]

Rebased on top of #495 to fix the RHD/LHD file detection. This adds back a few rules that were previously detected as redundant adjacencies (since both RHD and LHD files had been loaded simultaneously). All in all, the changes between the previous state of the branch and the current branch are small though:

130 files changed, 8833 insertions(+), 6910 deletions(-)

With the driveside issue fixed, the redundant-adjacency checker finds about 5k rules that are redundant in RHD only. (Most likely this means that the rule in question should be RHD-only, or a LHD rule for the base ortho/diag override is missing or broken.) I left these untouched, as there are only a few of them. Chances are that removing entire adjacency files resolves many of them.

I'm wondering if maybe it would be easier to verify that the correct basic overrides exist in the FLEXRamp code and then just delete the corresponding ramp adjacency files? We know that these files contain adjacency involving top, bottom, or branch ramp tiles, so we just need to check that the special case (adjacency to base ortho or base diag) is intact elsewhere. In fact, this is where a script to check for identical rules would be helpful. Then, we could manipulate the loading order to ensure FLEXRamp loads before the ramp adjacency files, and tag/delete the duplicate lines in the adjacency. That would give us the confidence to just delete the whole file.

Yeah, I think this would be do-able, now that the RHD/LHD issues is fixed. My expectation would be that only a very small number of rules is duplicated across all the FLEXRamp files and that the base ortho/diag code exists separately.

Would you like to remove the files that you already identified as dispensable?

[memo33]

@jflann On branch duplicate-overrides (5000afb atop #495), I've updated the conflicting overrides checker, adding an option to also identify purely duplicate override rules (that aren't conflicting).

On the branch here, it finds about 280k duplicates, spread across the entire repository. Unsurprisingly, in many cases, the duplicate is equivalent to the rule directly above it, just using different orientations. LHD/RHD code is also prone to duplicating code if the code isn't split into driveside-specific files (which is a fairly recent addition of the controller compiler). By far the most duplicates are in the file Controller/RUL2/07_RHW/Sec7m_Crosslinks/Sec7m_Crosslinks.txt: 57826 duplicates of a total of 74050 override rules. Not sure what's going on there.

I'm debating whether it makes sense to run the script on the branch here and commit, just so that it provides more information, making it easier to delete something. Not sure. It would also increase chances of merge conflicts with independent RUL2 branches.

[jflann]

I'm debating whether it makes sense to run the script on the branch here and commit, just so that it provides more information, making it easier to delete something. Not sure. It would also increase chances of merge conflicts with independent RUL2 branches.

I can see why we might hesitate to do something that's pretty likely to cause a conflict. However, I'd consider the fact that the conflicts would just be comments, so resolving them in favor of the actual work would be a no-brainer. Plus, all the deleting we are doing is bound to cause some conflict anyway.

I just checked how badly conflicted my own feature branch, meta-sam, is with staging. There are some conflicts but it can still be quickly rebased. There might be others out there that would be more difficult to deal with. I think it will be good to have that info on this branch and worth the risk.

[jflann]

Actually, on second thought I've changed my mind. I'm afraid it might make too much of a mess. I've had a look at the results of such a scan locally and I'm surprise at the extent of the duplicate code. It is basically everywhere.

[TarkusSC4]

By far the most duplicates are in the file Controller/RUL2/07_RHW/Sec7m_Crosslinks/Sec7m_Crosslinks.txt: 57826 duplicates of a total of 74050 override rules. Not sure what's going on there.

If the crosslink network's own RUL2 sections are set up properly, the similar bits in Sec7m_Crosslinks.txt should be redundant, as is the case with Line 811, which handles GLR. Sec10a_GLR1.txt has the exact same line at Line 2464.

0x57001700,0,0,0x08031500,1,0=0x57001700,0,0,0x5f880000,1,0

The other orientations of similar setups, i.e. where Diagonal GLR is extending out of a crossing with Orth RHW, don't have duplication in the GLR files, i.e., Line 3293:

0x57004700,2,0,0x08001a00,1,0=0x57004700,2,0,0x5f880600,3,0

But arguably, that kind of code really should be long GLR-side rather than RHW-side (i.e., in Sec10a instead of Sec7m).

It's also possible that some of the mid-adjacency code (i.e., to handle carrying a GLR override across multi-tile RHW network setups) might also be getting alternative coverage by virtue of the overrides to carry those RHW networks across GLR, too.

[TarkusSC4]

Also, a thought on the whole notion of the former auto-L1/L2 RHW-2 coming out of HTs/OSTs . . . the big reason it was killed off was the insane number of overrides it was going to take. I'm honestly curious if the DLL's surrogate adjacency handling might allow that functionality to be re-enabled without having to accept the tradeoff of insane amounts of code vs. unacceptable instability. Based on how many questions we get from the general public as to why the auto-L1/L2 doesn't exist anymore, I kind of have the impression it's largely viewed as a necessary evil at best among our users.

[memo33]

The branch is in a solid shape now. Some of the recent changes:

• removed the remnants of RHW and NWM adjacency code that are not needed anymore (about ~250k lines of RUL2 and 4.5 million lines worth of comments). This makes the entire repository significantly smaller and faster to work with.

• implemented a ReverseResolver which maps an ID to its corresponding metarule tiles. In the sbt console, it can be used interactively using the preimage function:

  preimage(0x57294745)
  // val result = List(L2Rhw8c~(0,+2,0,-2) & Glr3~(3,0,0,1))

• implemented a SegmentOrientationChecker script. It uses the reverse resolver to map RUL2 code from plain IDs to metarules where possible. It then runs several sanity checks on the metarule tiles and their segments in order to assert that there aren't any unexpected mismatches in direction or orientation. For example, if a rule accidentally reverses the direction of an ARD-3 segment or if the output tiles don't connect properly (in a loose sense), this would be detected.

  The script found roughly 100k buggy overrides so far. I've added the script to the GH actions, so it will prevent these kind of errors going forward. As more and more ID definitions are added to the metarule resolvers, the script will even get stronger over time.

• manual tweaks of RUL2 code here and there. In particular, I've removed all prevents intended as auto-connect killers.

With that, I think the branch is ready for merging and inclusion in the next build. Note that I've already merged the meta-sam branch here. I've also done some testing already, but getting more people to test, especially the more obscure features and edge cases, will be essential to be able to find and fix any remaining issues going forward.

One of the parts of RUL2 that's not quite in shape yet is the Viaducts code. There aren't enough comments to identify the adjacencies in there, so I'm inclined to leave the code as is, for now. Most likely, it will eventually be fully replaced by metarules, anyway. Even just adding more of the Viaduct IDs to the metarule ID resolver will help detect many buggy overrides, by means of the SegmentOrientationChecker script. That can wait for a new PR though.

[memo33]

I've made a few more tweaks. In particular, I went ahead and implemented the Viaducts IID scheme as a resolver, which allowed me to identify some more buggy overrides in the Viaducts RUL2 code. Now I'm done here.

[jflann]

Truly a massive amount of hard work on this branch. I am excited to see the results integrated into our next build. I'm good with merging this now.

[memo33]

Alright, let me go ahead and merge this then. Thanks a lot for your support on this.