Soundofdarkness / RuneBook

📖 Arcane Rune Pages manager for League of Legends 📜
MIT License
100 stars 21 forks source link

remove static rune mapping #45

Closed ghost closed 4 years ago

ghost commented 4 years ago

This PR removes the static mappings of the runes.

To do this, the current version of Riot is pulled at startup and processed by two new functions, which should make maintenance easier.

Notes:

PS: We should think about saving the Champ information and rune information locally so that we don't have to pull it every time we need to. Since they won't change during the patch anyway. (But that has nothing to do with this PR)

TinyRaindrop commented 4 years ago

Amazing, this has been bugging my perfectionist brain for too long.

Would this be a logical next step? Force recheck for new images whenever game version changes. So that users get new data even before a new Runebook update is released.

Soundofdarkness commented 4 years ago

Looks good so far, will test this when im able to play league a bit, and that force recheck generally sounds like something useful to add later on, we should just take care of handling problems with ddragon updating later than the patch happens.

ghost commented 4 years ago

@TinyRaindrop There are still some ideas. But let's discuss them in an issue :)

@Soundofdarkness Well currently you have to update Runebook. And I guess DDragon will always be faster than us.

And why do you have to play LOL to check the PR? :D

Soundofdarkness commented 4 years ago

@Rerago Yeah, well, I thought I can just change the runes through and check that all mappings fit (probably because of my distrust to ddragon due to numerous issues), but I guess I can just check a few later with opening random runepages and that should be enough. Will do that in a few hours.

Soundofdarkness commented 4 years ago

Yup, just tested it, from all I can tell without trying every rune it works beautifully, thanks :) (and sorry for my first brain fuckup)

Soundofdarkness commented 4 years ago

Pulled the first idea into an issue:

46