meraki-analytics / lolstaticdata

Accurate League of Legends static data for champions and items
MIT License
63 stars 22 forks source link

Added cdragon description to items #61

Closed rdavis0 closed 1 year ago

rdavis0 commented 1 year ago

Adds descriptions like the following to each item: "description": "<mainText><stats><attention> 250</attention> Health<br><attention> 35</attention> Armor<br><attention> 250</attention> Mana<br><attention> 20</attention> Ability Haste</stats><br><br> <active>Active -</active> <active>Conduit:</active> Designate an <attention>Accomplice</attention>.<br><li><passive>Convergence:</passive> After you <status>Immobilize</status> an enemy, your <attention>Accomplice's</attention> Attacks and Ability hits apply additional damage to that enemy.<br><br><rules>Champions can only be linked by one Zeke's Convergence at a time.</rules></mainText><br>",

jjmaldonis commented 1 year ago

Still pondering this one. Can you give more info about why this is valuable to you, what the tradeoffs are, and/or what were the considerations when adding it?

rdavis0 commented 1 year ago

Definitely! Currently, the only description in the item data is the simpleDescription which has very basic information at best (e.g. "slightly increases move speed"), or in many cases, nothing at all. Individual item effects have a description, but they're cluttered with Wiki formatting that would be difficult to parse through.

This added description is nearly a plaintext summary of the item's stats and effects. It comes straight from ddragon, and seems to be the standard for displaying item data across various third party tools and sites; it also matches the in-game description, which may be a plus for some users of lolstaticdata. Additionally, it can easily be plugged into html, and displays as one would hope, without any formatting or tags to work around. For me personally, this turned the prospect of displaying item effects on my site from an intensive regex endeavor to more or less just inserting the description.

For future reference, where's the best place to get input on changes like this ahead of time?

jjmaldonis commented 1 year ago

If you want to get input on changes like this ahead of time feel free to make an issue in this GitHub repo or message me on Discord. You can find me, Jason, in the Admins user list in the Riot Games Third Party Developer server.

Here are a handful of the questions running through my head:

Like you said, besides for the stats data, the items data only has a simple description and an icon. Adding a full description field is different than the main intention of the item data, which is to provide accurate stats for items and champions since that data does not exist elsewhere. So does a full description even belong in this data set? (As a side note, we probably shouldn't even be including the simpleDescription in this JSON.)

If the description is in ddragon and it's accurate, why do we need to reproduce it in lolstaticdata? Chances are, anyone who is using lolstaticdata is also using ddragon. If we include duplicate information, it's simply unnecessary bandwidth because it's providing exactly the same data.

The description data is not small, so it may increase the size of the JSON by ... 10%? That's non-trivial, but not the end of the world. Still, if it slows down load times by 10% when parsing this data that may be a pain. We don't want to bloat the data.

The description includes HTML tags. This makes it useful for people like you who are working on web pages. What if someone is not building a web/HTML app? Do they really want the descriptions in HTML format, or would they prefer the descriptions as a standard string with normal line breaks? They would probably prefer the non-HTML version, which means they would have to reformat the description and render the HTML into untagged text. That typically requires a library to do. So should this description be HTML or not? It depends on the use case! So lolstaticdata would either need to support only one use case, support both, or support none. If we support both, now we are definitely increases the size of the JSON by a non-trivial amount. Supporting only one doesn't seem like a good idea because it's biasing the data for a specific use case, which in my experience leads to the data being not-very-useful for other use cases and leads to branching of the data / data format and, eventually, multiple versions of the data being hosted by different people for different purposes. That just confuses things for everyone, and leads to people not using the data because it's impossible to know 1) who hosts the data, 2) which versions of the data are kept up to date, and 3) which version(s) I as a user should be using. It's just too much overhead for people to have to think about, and the chaotic nature of the environment pushes people to "build their own", which just exacerbates the problem. You've likely seen this plenty of times in other parts of the software ecosystems. There are a lot of "if then if then if then" statements in this paragraph, but these are the kinds of things we want to think about in terms of providing data for other applications to use and being a contributor to the League dev community. Our goal is to provide simple, easy-to-use data that is relevant and fills a direct need by the community. We also want the data to be useful to as many people as possible, and that requires some sacrifices. So it's a balance.

If this data is already the standard for displaying descriptions about items on various websites, then people must already have easy access to it -- sounds like that's ddragon and people are already finding it there. Adding a new source for that information conflicts with what I wrote in the last paragraph: if there are two sources for the same information, which one is the correct source to choose? People are going to ask themselves this question, and the correct answer is they should get it from ddragon not from lolstaticdata since lolstaticdata will be updated after ddragon. It is therefore a strictly poorer data source for this description. With that in mind, people will likely prefer to get the description from ddragon. And since they are likely using ddragon already, what's the need to provide it here? The only pro is that the information is slightly easier to get for the small subset of users who do not use ddragon. And the tradeoff is that we are: fracturing the community by rehosting the same data, increasing the JSON size, making the data more useful for HTML apps than other apps, and making it less clear what the vision/intention is for this data.

So it seems like there are lots of cons, and the benefit is pretty minor.

After writing all that, it seems pretty clear that we want to exclude this data for now.

rdavis0 commented 1 year ago

Makes sense. I guess I was misinterpreting the purpose of lolstaticdata as a one-stop shop for champ and item data, but it seems like the better approach is to use it in tandem with ddragon/cdragon. Appreciate your detailed response.

jjmaldonis commented 1 year ago

And thanks for all of your PRs, they are really helpful. I know other leople use them and appreciate it as well.

I will also say that so much of this is subjective, that it's hard to draw a line. Definitely feel free to ping me on discord or something if you want to talk about other stuff without spending the time to write a full PR.