search

Skretzo / shortest-path

Pathfinding for Old School RuneScape
BSD 2-Clause "Simplified" License
14 stars 27 forks source link

Add support for spell teleports #77

Closed gkotas closed 1 month ago

gkotas commented 2 months ago

Awesome plugin that was missing a very useful feature. By checking which runes/staffs are in the inventory and which teleports are unlocked (either magic level or quests), teleports can greatly reduce the shortest path.

Example

Here is an example of the shortest path to the Varrock bank from Arceuus. It uses the fairy ring and takes 144 steps when spell transports are disabled.

varrock

With them enabled, its just 33 steps.

varrock_spells

I added the display text to show the player which teleport to do.

varrock_spells_text

Also, I added checks for which spellbook is active. Using the standard spellbook, Dranynor is 164 steps.

draynor_standard

When switched to the Arceuus spellbook, the Draynor teleport can be used (assuming the required runes and magic level is met)

draynor_arceuuspng

How its done

I did this change in two parts. I have some example PRs on my forked repo to better show the changes along with some comments I made to help explain.

The first part was refactoring how the tsv file are parsed. Currently, all TSV files need the same columns in the same order even though they all don't need them. I changed this to use a map to map the header column name to the column value. This makes the parsing much simpler and the TSV files can be cleaned up.

The second PR is the actual spell teleport implementation. This is based on the TSV parsing refactor so my example PR is easier to review.

Next Steps

Currently the number of runes in the inventory is not checked so if a spell requires 3 runes and only 1 is in the inventory, the shortest path will still consider it valid. I have an idea to fix this in the future but its a minor detail as I would think most people have a ton of runes on them if they want to teleport.

There is a column in the existing TSV files called menuOption menuTarget objectID. I didn't change but it is not parsed anywhere. I assume this is left over from an older change and I would like to remove it if its not needed, but its not harming anything if it stays.

Brandon1212b commented 1 month ago

Is there away to get this added to the plugin or for us to use this feature?

Skretzo commented 1 month ago

Thank you for adding support for teleportation spells and rewriting the transport data parsing. I have been testing this for a few days now and it seems to work nicely. I will add some changes of my own and soon release a new update to the plugin with this new feature.