Cleanup item data creation and parsing#116
Conversation
…eation_and_parsing
| -- | ||
| -- Observed structure so far: | ||
| -- item = { | ||
| -- affixLimit = 0, -- usually 4 (most items) or 2 (idols). Is this useful? Should this be part of baseItem? |
There was a problem hiding this comment.
The affixLimit is used when crafting/editing an item, but it can indeed be moved to the baseItem
| -- affixLimit = 0, -- usually 4 (most items) or 2 (idols). Is this useful? Should this be part of baseItem? | ||
| -- affixes = {}, -- affix text with type and value identification. Ex: "{prefix}{range:31}{rounding:Integer}(6-17)% increased Health Regen". Extra entries or metamethods to get affix by type might be useful. Ex: item.affixes.suffixes | ||
| -- base = {}, -- base item data reference | ||
| -- classRequirementModLines = {}, -- unsure, shouldn't this be on requirements? |
There was a problem hiding this comment.
It can be removed (leftover from POB)
| -- classRequirementModLines = {}, -- unsure, shouldn't this be on requirements? | ||
| -- compatibleAffixes = {}, -- table of compatible modId for crafting (and validation?) | ||
| -- corrupted = false, -- corrupted flag | ||
| -- crafted = false, -- unsure, seems like leftover from PoB |
| -- compatibleAffixes = {}, -- table of compatible modId for crafting (and validation?) | ||
| -- corrupted = false, -- corrupted flag | ||
| -- crafted = false, -- unsure, seems like leftover from PoB | ||
| -- enchantedModLines = {}, -- list of enchanted mods texts, possibly with parsed results. Are these for enchanted idols or leftovers? |
There was a problem hiding this comment.
It can be removed (leftover from POB)
| -- corrupted = false, -- corrupted flag | ||
| -- crafted = false, -- unsure, seems like leftover from PoB | ||
| -- enchantedModLines = {}, -- list of enchanted mods texts, possibly with parsed results. Are these for enchanted idols or leftovers? | ||
| -- grantedSkills = {}, -- leftover from PoB, but might be useful |
There was a problem hiding this comment.
I don't think it will be useful to keep it
There was a problem hiding this comment.
What I was imagining was situations where an Item can enable a skill proc that you might not have otherwise, so having a reference to at least the skill ID so that it can be enabled on DPS calcs could be worthwhile.
Ex: Pillager's Gold or Kuzon's Fury that add ways to proc Burning Dagger
There was a problem hiding this comment.
The thing is that the mod (like "(35% to 50%) chance to throw a Burning Dagger on hit (up to 2 times per second)") will be parsed, and it will automatically make the references needed. I think that I changed a bit the logic for the grantedSkills, and I don't need this extra field on item atm
There was a problem hiding this comment.
Sounds good then!
| -- crafted = false, -- unsure, seems like leftover from PoB | ||
| -- enchantedModLines = {}, -- list of enchanted mods texts, possibly with parsed results. Are these for enchanted idols or leftovers? | ||
| -- grantedSkills = {}, -- leftover from PoB, but might be useful | ||
| -- id = 0, -- item id? where does this come from? |
There was a problem hiding this comment.
This one is necessary, I think, to reference the items when they are equipped. And to save/load the equipment.
There was a problem hiding this comment.
Aaaah that explains it! Yes, it makes total sense.
| -- raw = "", -- full string representation of the item data, useful for direct edit | ||
| -- requirements = {}, -- requirements to use the item {level = int} only saw level, but there should be class too | ||
| -- title = "", -- unsure whats the difference from name | ||
| -- type = "", -- item type string. Examples: "One-Handed Axe", "Shield", "Helmet", etc. Maybe should be a constant reference? |
There was a problem hiding this comment.
A constant reference should be good 👍
|
Good luck with the cleanup 🙂. Any simplification of the existing logic is welcome. Be careful not to oversimplify, as some variables like |
Yes, I'm presuming that's the case, and plan to do some extensive testing when this becomes ready. If necessary I can add some metamethods to ensure compatibility |
…eation_and_parsing
…eation_and_parsing
2 participants
This is still a WIP. I want to have it readable as a draft PR to facilitate discussion.
Description of the problem being solved:
Currently the code around item parsing is a bit hard to read/understand, and the ItemClass is populated with many duplicate fields while also being slightly complicated to identify some basic info about the item. Ex: When checking a Shield item its not super intuitive on how to get the base (implicit) block chance, which is what spurred me into diving into this section of the code.
There's also some limitations currently on how to identify corrupted items during the offline save import, so I'm trying to investigate that too.
Steps taken to verify a working solution:
Link to a build that showcases this PR:
Will add later on
Before screenshot:
After screenshot: