Rhyono / CraftStore

ESO Addon
7 stars 11 forks source link

Question about CS.GetTrait() #54

Open Sharlikran opened 2 months ago

Sharlikran commented 2 months ago

@Rhyono

  local trait, eq, craft = GetItemLinkTraitInfo(link), GetItemLinkEquipType(link)
  if not CS.IsValidEquip(eq) or not CS.IsValidTrait(trait) then
    return false
  end
  local at, wt, line = GetItemLinkArmorType(link), GetItemLinkWeaponType(link), nil
  if trait == ITEM_TRAIT_TYPE_ARMOR_NIRNHONED then
    trait = 19
    if at == ARMORTYPE_NONE then trait = 19 end
  end -- Nirnhoned Weapon replacement
  if trait == ITEM_TRAIT_TYPE_WEAPON_NIRNHONED then
    trait = 9
    if wt == WEAPONTYPE_NONE then trait = 19 end
  end -- Nirnhoned Armor replacement

If you have Nirnhoned armor you change the correct trait the API already returned to, ITEM_TRAIT_TYPE_ARMOR_ORNATE? When it's ARMORTYPE_NONE but Nirnhoned, you set the trait to ITEM_TRAIT_TYPE_ARMOR_ORNATE.

If you have Nirnhoned weapon you change the correct trait the API already returned to, ITEM_TRAIT_TYPE_WEAPON_INTRICATE? When it's WEAPONTYPE_NONE but Nirnhoned, you set the trait to ITEM_TRAIT_TYPE_ARMOR_ORNATE.

What is the preference or purpose of a Nirnhoned Weapon or Armor being set to Ornate or for that matter Intricate?

Why? Better question why are you using integers and not the game constants. Anyone picking up this project would have no idea what the seemingly arbitrary integers are supposed to be doing.

Later

  elseif wt == WEAPONTYPE_SHIELD then
    craft = 6
    line = 6
    trait = trait - 10

For a shield with ITEM_TRAIT_TYPE_ARMOR_IMPENETRABLE you subtract 10 which would make it ITEM_TRAIT_TYPE_CATEGORY_ARMOR. Then if it is ITEM_TRAIT_TYPE_ARMOR_INFUSED that makes it ITEM_TRAIT_TYPE_WEAPON_TRAINING. There are others like ITEM_TRAIT_TYPE_ARMOR_REINFORCED becomes ITEM_TRAIT_TYPE_CATEGORY_JEWELRY.

What was the intention behind that?

Then the same for ARMORTYPE_HEAVY, ARMORTYPE_MEDIUM, and ARMORTYPE_LIGHT also have 10 subtracted from the trait the API returned. Which changes ITEM_TRAIT_TYPE_ARMOR_WELL_FITTED to ITEM_TRAIT_TYPE_WEAPON_INFUSED.

Sharlikran commented 2 months ago

I have to add ITEM_TRAIT_TYPE_JEWELRY_BLOODTHIRSTY is changed to ITEM_TRAIT_TYPE_WEAPON_INTRICATE.

And then I noticed you change the equip type by adding 7 depending on where the item is equipped such as the waist. So that makes EQUIP_TYPE_WAIST become EQUIP_TYPE_POISON.

There are a few places where CS.GetTrait precedes CS.IsItemNeeded() so if a waist item is changed to a poison item, which I don't think you can research, that doesn't make sense.

Sharlikran commented 2 months ago

What was the intention of all that? Because whatever it was the constants have changed enough most likely that you don't want to be doing that anymore or at least use the games constants for whatever that is intended for.

Granted at the moment, the line and trait are changed the same way such that any player's saved data should return something CraftStore is looking for. However, what is happening doesn't make any sense.

Sharlikran commented 2 months ago

In addition to all that the constants on the Wiki go to 33 and it's now 60.

Sharlikran commented 2 months ago

I updated the Wiki so no idea whether or not what I explained above is the same now. Anything under 33 might be the same or close to the same.

Sharlikran commented 2 months ago

Also CS.IsValidTrait is excluding Intricate or Ornate for Armor, Weapon, and Jewelry.

  local trait, eq = GetItemLinkTraitInfo(link), GetItemLinkEquipType(link)
  if not CS.IsValidEquip(eq) or not CS.IsValidTrait(trait) then
    return false
  end

Which makes sense because you are checking to make sure it is a valid equip location and not Intricate or Ornate.

Then why when it's Nirnhoned, and armor or weapon type none, do you set it to Intricate or Ornate? If it's Nirnhoned and you find it in the overland, would it ever be Intricate or Ornate? If someone crafts one for research would it ever be Intricate or Ornate? Can someone even craft Intricate or Ornate and if not then why does that matter for CS.GetTrait()?

The name of the function is GetTrait() but what is the intention?

GetItemTraitInformationFromItemLink() can return if the item can be researched or not. It does not return ITEM_TRAIT_INFORMATION_CAN_BE_RESEARCHED if the item simply could be researched but the character already researched the item type and the specific trait.

So GetItemTraitInformationFromItemLink() will only return ITEM_TRAIT_INFORMATION_CAN_BE_RESEARCHED if the item can be researched by the character.

GetItemTraitInformationFromItemLink() isn't currently even used in CS. So if you have 10 functions and hundreds of values in saved variables for each character you could eliminate all that using that function. Storing the information for each character would still be needed once you have switched to another character to better manage items.

Zeratoxx commented 2 months ago

@Sharlikran You misunderstand the role of the variable trait in your first message I assume. The variable is not about the actual ingame trait IDs, it's about the according row of the table of craftstore ^^

Zeratoxx commented 2 months ago

@Sharlikran so the table has always 9 rows. You need to somehow match the trait id with the trait row of the table.

image

Zeratoxx commented 2 months ago

Naming is probably not clear ^^

Zeratoxx commented 2 months ago

A more robust solution would be a simple and dumb "if ITEM_TRAIT_TYPE_ARMOR_NIRNHONED then row=9" and do that for all other traits. If doing this, the addon would be unaffected by ID changes from ZOS bc the constants would always fit to the ingame data.

Sharlikran commented 2 months ago

If trait is what you mentioned, then what is line?

Zeratoxx commented 2 months ago

line is column :D

As I said,

Naming is probably not clear ^^

Sharlikran commented 2 months ago

image

I'm looking into things with the UI dev and Dolgubon because for WritWorthy it was a much easier fix. Just reversing 1 and 2.

    -- research lines
,   RING                =  2
,   NECKLACE            =  1

So for CS

line = (eq == EQUIP_TYPE_NECK) and 1 or 2

However, the tooltip is still wrong for me at least.

Zeratoxx commented 2 months ago

"reversing 1 and 2." was also the fix here logic wise, but CraftStore works with persistent inventory data while writ worthy doesn't. The existing data needs to get fixed actively.

The migration routine won't get kicked off correctly it seems.. I already saw that no chat messages get sent. I thought that the routine gets executed with effect but it gets called too early when the game got freshly started it seems, which results in the migration routine not having an effect.

Meh.

Sharlikran commented 1 month ago

I can run it manually and when I did on character that doesn't have researched traits it said nothing to do on everything.

Zeratoxx commented 1 month ago

I guess you activated the debug mode

Zeratoxx commented 1 month ago

I can run it manually and when I did on character that doesn't have researched traits it said nothing to do on everything.

Could you explain more in detail on what you did? ^^

Zeratoxx commented 1 month ago

@Sharlikran how about now? Did anything change for you after the newest update?

Sharlikran commented 1 month ago

I haven't tried it because I have been working with someone else and asking my peers as well as Dolgubon. I'm looking into other things at the moment.