Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attribute names in /v2/items #112

Closed
dr-ishmael opened this issue Nov 3, 2015 · 9 comments
Closed

Attribute names in /v2/items #112

dr-ishmael opened this issue Nov 3, 2015 · 9 comments
Labels

Comments

@dr-ishmael
Copy link

The new attributes Concentration and Expertise are being returned on the items endpoint attributes and buff as BoonDuration and ConditionDuration, the same as actual Boon Duration and Condition Duration. Luckily, on equipment (weapons/armor/trinkets/back) they can be distinguished because Concentration/Expertise actually show up in attributes, whereas Boon/Condition Duration only show up in buff.

However, upgrade components only have a buff, so we have to look to see if the number has a percent sign or not before we can translate Boon/ConditionDuration to Concentration/Expertise.

Is there any way these attribute names can be updated?

PS: the attribute Ferocity is still being returned as CritDamage, although this isn't a major problem because it doesn't create any conflicts.

@lye
Copy link
Contributor

lye commented Nov 3, 2015

I mean, they're enumeration values that have been there for a long time, so we can't change them without breaking existing applications. The Boon/ConditionDuration enum value has been in there for awhile; I'm not sure if it's entirely safe to change.

The "Boon Duration +X%" buff is the old-style before it was set up as an actual attribute -- it was implemented as a hidden buff that modified boon/condi applications when you used a skill. When we added durations as actual attributes, the old stuff didn't get updated. There are two completely separate ways to modify boon/condi durations, and the API reflects this. I agree that it's kinda gross, but I don't really want to mess with it until I sneak /v2/itemz past @tivac.

@dr-ishmael
Copy link
Author

I understand that about the buffs - back when Magic Find was an equipment
attribute, it was built the same way. But you're saying that the percentage
Boon Duration and the non-percentage Concentration share the same
enumeration?

On Tue, Nov 3, 2015 at 12:27 PM, lye notifications@github.com wrote:

I mean, they're enumeration values that have been there for a long time,
so we can't change them without breaking existing applications. The
Boon/ConditionDuration enum value has been in there for awhile; I'm not
sure if it's entirely safe to change.

The "Boon Duration +X%" buff is the old-style before it was set up as an
actual attribute -- it was implemented as a hidden buff that modified
boon/condi applications when you used a skill. When we added durations as
actual attributes, the old stuff didn't get updated. There are two
completely separate ways to modify boon/condi durations, and the API
reflects this. I agree that it's kinda gross, but I don't really want to
mess with it until I sneak /v2/itemz past @tivac
https://github.com/tivac.


Reply to this email directly or view it on GitHub
#112 (comment).

@tivac
Copy link
Contributor

tivac commented Nov 3, 2015

but I don't really want to mess with it until I sneak /v2/itemz past @tivac.

ಠ_ಠ

@sliekens
Copy link

sliekens commented Nov 3, 2015

ಠ_ಠ

ಠ◡ಠ

@lye
Copy link
Contributor

lye commented Nov 3, 2015

But you're saying that the percentage Boon Duration and the non-percentage Concentration share the same enumeration?

Nah, the percentage Boon Duration is not a character attribute. It's not even an enumeration value -- I'm pretty sure it's just a string (it's even localized!). :sadness:

EDIT: Apparently there are three ways in-game that this is communicated...

@dr-ishmael
Copy link
Author

Interesting. I'm at home now and checked some of these in-game - the upgrade components, like Freshwater Pearl and Exquisite Freshwater Pearl, actually show +9 Boon Duration, just like it is in the buff on the API. But the trinkets like Freshwater Pearl Orichalcum Earring, get transformed to +35 Concentration.

And NOW I read your edit and see that you already know about it. Ah well, I took the time to write all that, I'm posting it anyway. :)

EDIT: The +10% Condition Duration shown on the Giver's Iron Axe is what that item has always had, and it does add 10% directly to Condition Duration (I just bought one in-game and equipped it.) So Giver's equipment should be fine, it's the new items with the non-percentage stats that appear to be flawed in their buff terminology.

@lye
Copy link
Contributor

lye commented Dec 7, 2015

I think what I want to do is add an additional field to the details object that contains the machine-readable jewel stats. I don't want to mess with the existing fields for fear of application breakage. I'll have to write a PR later.

@Archomeda
Copy link
Contributor

Recently the Boon Duration and Condition Duration stats on (as far as I know, all) equipment have been changed to their non-percentage equivalent, so I think this is no longer an issue?

@Aonwy
Copy link

Aonwy commented Mar 26, 2018

If there is additional need around this, please write up a new ticket.

@Aonwy Aonwy closed this as completed Mar 26, 2018
@ghost ghost removed the ready label Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants