Missing Ornn Items

[vorpal56]

Ornn Items are not included in the Item data from DDragon, but do exist in the game. But, since the Item data is not included in DDragon, we're missing item images eg. Sandshrike's Claw, ID=7000. The Ornn Item ID's are between 7000 - 7022 (inclusive). Looking at the CDragon data as well, fields like requiredAlly should be set to Ornn, active should be set to true, and store should be set to false. This means that the some fields are incorrect and other fields like icon in the Item model would be unpopulated. These items are edge cases, but I think these should be added in the item set.

The general steps I would do is:

1. Get all the item items that are forgeable by Ornn in his ability passive description
2. Create a dictionary of Ornn items using CDragon i.e. {7000: "Sandshrike's Claw",...}. CDragon Ornn item names follow %i:ornnIcon% <item_name>. So if the name matches this expression, we know it's an Ornn item.
3. Loop through the Ornn items and follow the same process as if it were a regular item.

Should these items be accounted for, and if so, what could we do about missing fields like icon and simpleDescription?

[vorpal56]

So in the initial steps I wrote above, there's no point in having to do step 1. We can look at step 2 and 3 directly (without the dictionary of Ornn items, just iteratively in CDragon). I've written down the code below (which will be in __main__.py. The only problem is it's just the same code repeated twice (it's exactly the same with for d in ddragon), we import re in __main__.py, and we have static entries for fields that we can't find details on in DDragon i.e. icon and simple_description:

import re # additional import

def _name_to_wiki(name: str):
  ... # unchanged function
def main():
  for d in ddragon:
    ... # unchanged steps
  for cdrag in cdragon:
    ornn_item_match = re.search(r"(?:%i:ornnIcon% )([A-Za-z0-9' \-]+)", cdrag["name"])
    if ornn_item_match is not None:
      ornn_item_name = ornn_item_match.groups()[0]
      item = _name_to_wiki(ornn_item_name)
      item.name = ornn_item_name
      item.id = cdrag["id"]
      item.icon = "" # what would we do here?
      item.builds_from = cdrag["from"]
      item.builds_into = cdrag["to"]
      item.simple_description = "All stats have been improved." # this is the caption of the item
      item.required_ally = "Ornn" # you can't forge ornn items without Ornn as your ally
      item.required_champion = cdrag["requiredChampion"]
      item.shop.purchasable = False # Ornn items aren't purchasable, Ornn forges them
      if item is not None:
        jsonfn = os.path.join(directory, "items", str(item.id) + ".json")
        with open(jsonfn, "w", encoding="utf8") as f:
          j = item.__json__(indent=2, ensure_ascii=False)
          f.write(j)
        jsons[item.id] = json.loads(item.__json__(ensure_ascii=False))
        print(item.id)
  jsonfn = os.path.join(directory, "items.json")
    with open(jsonfn, "w", encoding="utf8") as f:
      json.dump(jsons, f, indent=2, ensure_ascii=False)
  del jsons
  return

if __name__ == "__main__":
    main()
    print("Hello! What a surprise, it worked!")

Any suggestions?

[jjmaldonis]

Awesome, this will be a great addition. Overall it looks good. As usual, I have a handful of small things that would be useful to update:

1. CDragon should have the icon data, e.g. /lol-game-data/assets/ASSETS/Items/Icons2D/6662_Tank_T4_FrostfireGauntlet.png. I don't remember the prefix but it's in Cass or in Pyot if not in Cass.
2. It's too bad CDragon doesn't have a simple description. Let's do something like f"An Ornn-updated item built from {item.name}". When people ask, "What item is that?" we want to use whatever answer they will be given, which in my opinion is what I wrote.
3. Let's use f-strings when we can, and please replace str(item.id) + ".json" with f"{item.name}.json".
4. Let's make sure to use black for formatting.
5. Please add a comment above if item is not None: because it is not immediately clear to me what the intention of that if block is.
6. The re import is no problem, it's built into Python and is part of the standard library.
7. Please capitalize the first work in comments. It's just a personal thing I like and I think it makes the code more readable. I haven't been super strict about that in the past but if you're making changes anyways I'd appreciate it.

Otherwise it looks good! It'd be nice if we could reduce the code duplication but this is a unique case so I'm okay with having duplicate code. If it becomes a problem, we can fix it in another iteration.

Thanks for your work here!!

[vorpal56]

Thanks for your help! I really do appreciate the structured way of contributing to the project, it keeps it consistent and concise for readers and contributors. To note the things you've talked about in the previous comment:

1. CDragon has the icon data for the original item, but the actual icon with the Masterwork overlay is not present. The path for the item icon images in CDragon are in https://raw.communitydragon.org/latest/plugins/rcp-be-lol-game-data/global/default/assets/items/icons2d/. Taking the Sandshrike's Claw (ID=7000) example, iconPath: "/lol-game-data/assets/ASSETS/Items/Icons2D/6693_Assassin_T4_ProwlersClaw.png" is the same as Prowler's Claw iconPath
2. Yeah, it's a shame, but I will change the simple description to the one you've noted. It makes much more sense than the temporary one I've set 👍
3. I took the same lines of str(item.id) + ".json" in line 81 of items/__main__.py. I will change that line as well to use f-strings.
4. Will do. I wanted it to fit better in the GitHub comments section
5. if item is not None: is the same as line 80, but I guess in the mentioned code, it's not necessary since there will always be a populated item. This will be removed as well.
6. Great! I thought that it wouldn't 'fit' in __main__.py
7. What do you mean about capitalizing the first work in comments?

I've also looked around and I found this exact issue on Riot Games/developer-relations. This wasn't an issue before the new items, but I guess as temporary fix we can include the above code.

Thanks again for your help!

[jjmaldonis]

Hmm, for the icons... they do exist in http://raw.communitydragon.org/10.24/game/data/items/icons2d/ but they are named terribly - you can ctrl+f for "ornn" and find some of them, but others don't have his name in the filename. They also exist on the wikia with a better formatted name: https://static.wikia.nocookie.net/leagueoflegends/images/5/52/Upgraded_Aeropack_item.png which you can find using the inspect tool from https://leagueoflegends.fandom.com/wiki/Ornn/LoL

If you want to set the icon to None for this PR that's fine, but we should link to the correct icon at some point.

"capitalizing the first work" should have been "capitalizing the first word". So # you can't forge ornn items without Ornn as your ally can be changed to # You can't forge ornn items without Ornn as your ally. Typically I follow all the styles in the Python Cookbook by David Beazley, and you'll note that his code snippets read more like a text document than "code", that's how well formatted they are.

Riot is typically slow to update DDragon, although they have been faster to update it when images are missing. It will be a few weeks at least I'd guess though.

[dryancd]

I already added ornn items in a branch on my fork. You don't need to add it.

[jjmaldonis]

@vorpal56 We talked on the discord about icon images, just leave it set to None for now. We'll fix it later.

[vorpal56]

@jjmaldonis Ok great. I'll have @dryancd handle the PR. Sorry for not looking at the files in CDragon more thoroughly, there's just so many different files and folders and I was just clicking through each one hoping I would find it 😅

[dryancd]

Nah, it's my fault @vorpal56 I didn't create a pr for it. You can still create a pr with your stuff and I'll compare it with mine and see if I need to fix anything.