search

evil-morfar / RCLootCouncil2

RCLootCouncil - addon for World of Warcraft
https://rclootcouncil.com
GNU Lesser General Public License v3.0
19 stars 29 forks source link

Uncached lootTable items #160

Closed evil-morfar closed 4 years ago

evil-morfar commented 6 years ago

There's a small potential for throwing away data with the current lootTable caching system. Consider this log from a recent raid of mine:

"20:15:50 - Comm received:^1^SlootTable^T^N1^T^N1^T^SequipLoc^SINVTYPE_HAND^Sawarded^b^Slink^S|cffa335ee|Hitem:152012::::::::110:104::5:3:3611:1512:3337:::|h[Molten~`Bite~`Handguards]|h|r^Srelic^b^Stexture^N1605809^SsubType^SPlate^SlootSlot^N3^Sclasses^N4294967295^Sname^SMolten~`Bite~`Handguards^Sboe^b^Silvl^N970^Squality^N4^t^N2^T^SequipLoc^SINVTYPE_TRINKET^Sawarded^b^Slink^S|cffa335ee|Hitem:151974::::::::110:104::5:4:3611:3618:1497:3336:::|h[Eye~`of~`Shatug]|h|r^Srelic^b^Stexture^N463857^SsubType^SMiscellaneous^SlootSlot^N2^Sclasses^N4294967295^Sname^SEye~`of~`Shatug^Sboe^b^Silvl^N955^Squality^N4^t^N3^T^SequipLoc^S^Sawarded^b^Slink^S|cffa335ee|Hitem:152291::::::::110:104::5:3:3611:1492:3336:::|h[Fraternal~`Fervor]|h|r^Srelic^SLife^Stexture^N459025^SsubType^SArtifact~`Relic^SlootSlot^N1^Sclasses^N4294967295^Sname^SFraternal~`Fervor^Sboe^b^Silvl^N950^Squality^N4^t^t^t^^ (from:) (Barrow) (distri:) (RAID)", -- [1356]
"20:15:50 - Comm received:^1^SlootAck^T^N1^SEliarra-Ravencrest^N2^N258^N3^N961^N4^T^Sresponse^T^N1^B^N2^B^t^Sdiff^T^N1^N25^N2^N5^N3^N0^t^Sgear1^T^N1^Sitem:152155::::::::110:258::5:3:3611:1487:3528^N2^Sitem:151962::::::::110:258::3:3:3610:1492:3337^t^Sgear2^T^N2^Sitem:151955::::::::110:258::5:3:3611:1492:3336^t^t^t^^ (from:) (Eliarra) (distri:) (RAID)", -- [1357]
"20:15:50 - Comm received:^1^SlootAck^T^N1^SValhorth-Ravencrest^N2^N262^N3^N964.3125^N4^T^Sresponse^T^N1^B^N2^B^t^Sdiff^T^N1^N-30^N2^N20^N3^N0^t^Sgear1^T^N1^Sitem:151819:5444:::::::110:262:::2:1811:3630^N2^Sitem:154177::::::::110:262::3:2:3985:3993^t^Sgear2^T^N2^Sitem:151955::::::::110:262::3:3:3610:1477:3336^t^t^t^^ (from:) (Valhorth) (distri:) (RAID)", -- [1358]
"20:15:50 - Comm received:^1^SlootAck^T^N1^SBenó-Ravencrest^N2^N251^N3^N960.5^N4^T^Sresponse^T^N3^B^t^Sdiff^T^N1^N25^N2^N-10^N3^N0^t^Sgear1^T^N1^Sitem:152114::::::::110:251::5:3:3611:1487:3528^N2^Sitem:137459::151583::::::110:251::35:4:3536:1808:1617:3337^t^Sgear2^T^N2^Sitem:154176::::::::110:251::3:2:3985:3991^t^t^t^^ (from:) (Benó) (distri:) (RAID)", -- [1359]
"20:15:50 - Comm received:^1^SlootTable^T^N1^T^N1^T^SequipLoc^SINVTYPE_HAND^Sawarded^b^Slink^S|cffa335ee|Hitem:152012::::::::110:104::5:3:3611:1512:3337:::|h[Molten~`Bite~`Handguards]|h|r^Srelic^b^Stexture^N1605809^SsubType^SPlate^SlootSlot^N3^Sclasses^N4294967295^Sname^SMolten~`Bite~`Handguards^Sboe^b^Silvl^N970^Squality^N4^t^N2^T^SequipLoc^SINVTYPE_TRINKET^Sawarded^b^Slink^S|cffa335ee|Hitem:151974::::::::110:104::5:4:3611:3618:1497:3336:::|h[Eye~`of~`Shatug]|h|r^Srelic^b^Stexture^N463857^SsubType^SMiscellaneous^SlootSlot^N2^Sclasses^N4294967295^Sname^SEye~`of~`Shatug^Sboe^b^Silvl^N955^Squality^N4^t^N3^T^SequipLoc^S^Sawarded^b^Slink^S|cffa335ee|Hitem:152291::::::::110:104::5:3:3611:1492:3336:::|h[Fraternal~`Fervor]|h|r^Srelic^SLife^Stexture^N459025^SsubType^SArtifact~`Relic^SlootSlot^N1^Sclasses^N4294967295^Sname^SFraternal~`Fervor^Sboe^b^Silvl^N950^Squality^N4^t^t^t^^ (from:) (Barrow) (distri:) (RAID)", -- [1360]

It's possible for lootAcks to arrive between the 1 frame recall to OnCommReceived. This causes the acks to be applied to a previous session, as VotingFrame:Setup() is called later, thus briefly showing candidates as "timeout" until their response arrives, but still doesn't show their equipped gear or potential autopass. It happens very rarely (I believe I've seen it ~5 times so far), but is an issue nonetheless.

As of right now, afaik, RCLootCouncil does not require any items to be cached, so the easiest solution is to do the GetItemInfo calls to prompt the cache, but without doing anything if it's not cached. This still caches (as per our tests) all items within the next frame - the only issue with that is if anything anywhere assumes the items are cached within that 1 frame.

A few other solutions

  1. Cache incoming lootAcks and probably also responses. This is somewhat tricky, as there's currently no way of knowing which lootTable a particular lootAck belongs to, which I believe is required to ensure it's only applied the right place/time.
  2. Delay everything but VotingFrame:Setup() at least 1 frame. This would hardly be noticeable for anyone, but is a delay, and somewhat annoying to do.
  3. Make sure everything that needs cached item info won't crash without it. This should honestly already be implemented, and imo is the best of the 3.
SafeteeWoW commented 6 years ago

3 completely solves the problem, but it requires a better design to make it work. It would suit for RC v3.0 Loottable is not a good structure to store the item information

However 2 is the easiest solution right now, but it does not completely solve the problem. It just makes the bug happen much less often, because there is no guarantee that item info is fetched within 1 frame (though it does most of the time).

1 is a bad design. I don't like it.

SafeteeWoW commented 6 years ago

As what I said in #164, to completely solve this issue, the entire code structure to handling item information should be changed.