HardstuckGuild / GW2-Tooltips.js

Tooltips.js frontend
1 stars 1 forks source link

'Overridden Because' is misleading in some rare cases #65

Closed mightyteapot closed 9 months ago

mightyteapot commented 9 months ago

Bitter chill causes chill to also apply vulnerability, in tooltips this manifests as 'overridden because of bitter chill' whereas it should actually say 'exists because of' image image

Interestingly, bitter chill behaves correctly on reaper's mark when combined with the reaper 'chill on fear trait' (Shivers of dread) image And fails to apply entirely when combined with 'Chilling Darkness' (chill on blind) image

Shivers of dread is a very similar trait and appears to work correctly, so it might be an issue with bitter chill specifically.

Build: https://beta.hardstuck.gg/gw2/builds/necromancer/condition-reaper/

SaculRennorb commented 9 months ago

Oh, that it extremely unfortunate. This bug has nothing to do with the trait displayed, its because of something different. Weather or not it said 'overriden...' or 'exists...' gets determined by something like

if(fact.required_trait) {
  if(fact.skip_next) show('overridden...')
  else show('exists...')
}

The problem with 'Suffer' is that this fact not only exists because of the trait, but there also is a split between gamemodes, as the pvp variant applies only 6s where the pve applies 8. This split uses the same system, so the order becomes

  1. wvw fact with 6s, exists because if trait, skip next
  2. normal fact with 8s, exists because of trait

this results in the pvp version of the fact being treated as 'overrides', because it actually overrides something. it overrides the pve version of itself.

I cannot simply test weather or not the overriden (next) fact is the same thing as the current one, as there are legitimate cases where this happens (the necro WH trait for example overrides facts with different versions of themself).

I need to think really hard about this one, i don't have a good solution as of now.

SaculRennorb commented 9 months ago

This should work now, but i haven't had may test cases to look at. Also The fix should not interfere with anything, but keep any eye out for the 'exists because..' now being to prevalent and being used where it should actually be 'overridden because...'

The combination not being properly processed on deathly swarm is an ingame issue: #69.