Transport Refactor

[FIrgolitsch]

I am starting work on refactoring the transport type logic so it's easier to extend when new transports need to be added.

[Skretzo]

Here is an example of what it currently looks like when I add a new transport type: c0b70f9.
The part that can be a bit confusing is that I need to do 4 things:

• Add a config option to ShortestPathConfig.java, usually a boolean. This should probably not be refactored.
• Add an entry to the TransportType.java enum
• Add a code line to loadAllFromResources() in Transport.java
• Add several changes to PathfinderConfig.java

[FIrgolitsch]

Thanks for the reference! That'll help. I haven't looked yet, but are there currently any checks for the availability of transports, or is the user's choice if they are enabled?

[Skretzo]

A possible refactor that can be explored is to move all the transports away from TSV files and over to separate enums. Enums in Java allow you to define several constructors and can support arbitrary number of arguments using e.g. int... whatever as last constructor argument.

[Skretzo]

I haven't looked yet, but are there currently any checks for the availability of transports, or is the user's choice if they are enabled?

A certain type of transport (e.g. all hot air balloons) will be considered during the pathfinding if:

• The associated config is enabled, e.g. useHotAirBalloons() from ShortestPathConfig.java returns true.

A specific transport (e.g. the hot air balloon from Varrock to Castle Wars) will be considered during the pathfinding if:

• There is an entry with a start location and end location in a TSV file, e.g. hot_air_balloons.tsv, sometimes with associated item requirements, skill requirements, quest requirements, unlock requirements (varbits, varplayers), etc..
• The player is passing the check of every associated item requirement, skill requirement, quest requirement, etc..

[FIrgolitsch]

Yeah, I was thinking along the same lines. Enums usually have my preference for this kind of thing, as I find them to be a bit more solid programatically. I do like the TSV for storing the data, as that can get a bit ugly in code. The enum constructors seem like a good place for that.

[FIrgolitsch]

I fixed the issues you laid out and changed a bunch of things again. One thing always seems to lead to another.

• The TSV files all now have the same number of columns, comments included. This actually was mostly already the case, they were just filtered out in the script.
• Removed the item requirements for the hot air balloons, they are working correctly now.
• Replaced the 'var' assignments with their specific type.
• Added the bank check for the Dramen/Lunar staff. This required some overhaul as this dependency is set at runtime not in the TSV files.
• After adding it I noticed there was a difference in routing between when the staff was in the inventory or when it was in the bank 1 tile away. There were two problems, the first being that transports were not properly updated after a bank visit was made. That has now been corrected.
• The second problem was that when BFS explores the path, it marked that a bank was visited as it explored the search space. This meant that no matter what the actual optimal path is, banks were visited as BFS would explore them. I fixed this by adding a bank visited flag to the Node class to keep track of banks on the current path only.
• Once this was fixed it properly routed trough chains of transports, requiring multiple items. In particular the Ardy cloak and the dramen staff. I've modified the transport hint to show the items needed below when they need to be picked up at a bank:
• This hint changes dynamically based on the inventory. For example. after taking out the Ardy cloak:
• Created a new class in the Transport package to consolidate this behaviour.
• Added a bunch of tests to verify the new logic for chaining transports and weird cases that popped up during testing. Some of these are end-to-end tests to check if certain conditions are met. For example, a route from castle wars to the fairy ring in the Piscatoris hunting grounds (AKQ). Before (in the current live version) it would send you to the Outpost and walk for about 200 tiles. Also added a test for checking that if the Ardy cloak is the inventory it would not first teleport to the monastery and then look for a bank, leading it the suggest to pick up a necklace of passage to the wizard tower instead (plus the staff), taking the fairy ring at the WT. It now waits for the item pickup at the closest bank. It's very specific for the fairy rings right now. It probably could be broader but I wanted to restrict the impact for now.

[mpickering]

As a first time contributor, I would have found it easier to modify part of the program rather than a TSV file since my normal configuration strips trailing whitespace and expands tab characters.