search

Soundofdarkness / RuneBook

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

Blitz.GG kinda broken #126

Open ghost opened 2 years ago

ghost commented 2 years ago

The plugin works only conditionally. See the Aram runes:

image

Soundofdarkness commented 2 years ago

Yup that's something I saw when testing too, but considering how the api exposes the runes im not sure if there is an easy fix, as they send multiple rune options with winrates and we have to decide what the right ones are. (And sometimes they even don't have two runes of the same secondary tree). So im personally not sure how to fix that (unless it's another issue)

Soundofdarkness commented 2 years ago

Alright, just checked the api response for that case, it seems like they actually report id 9111 for both slots (together with others, but highest wr are both that id). So I guess the only option would be to disallow duplicates, but I'm not sure if that would properly fix it, or just move the issues to somewhere else. image (index 3 and 4 both have the same runeId )

ghost commented 2 years ago

Well, in the end, the page gets no other answer. You would just have to look at how the page processes that:

https://blitz.gg/static/js/main.7c37e4ed.chunk.js

Soundofdarkness commented 2 years ago

Technically yes, but at least personally I don't think I'm going to reverse engineer half-minified js that seems to also include parts of their app for it.

ghost commented 2 years ago

There app itself is also just displaying the website as far as the runes are concerned. The logic regarding the runes calculation seems to be in this JS. I did not want to do that either, I just wanted to note it does not have to be impossible :D

Soundofdarkness commented 2 years ago

Yep, absolutely. I'm guessing it's possible to jump back if something is a duplicate and take the next highest winrate one, but that feels a lot like something that could break extremely easily.

ghost commented 2 years ago

That is, there are not only problems with duplications but also with "impossible" rune combos. Look here:

Yasou ADC yasou

There "Overgrowth" and "Revitalize" is set. But this is not possible because these are in same row.

89Q12 commented 2 years ago

Its also possible that I choose wrong params for the graphql endpoint. I'm gonna look into it now