Infinitay / essence-pouch-tracking

BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Fix incorrect state when RCing fast (#13) #16

Closed Infinitay closed 1 week ago

Infinitay commented 1 week ago

Fixes #13

Infinitay commented 1 week ago

I iteratively tested the changes for 10 minutes at Astral's and didn't have an issue. I then went to test it longer while doing GOTR for around 30-40 minutes. Again, I couldn't actively recreate the issue where emptying the pouch returned an incorrect state like before when runecrafting and trying to empty the pouches too quickly. However, it did happen to me just once where one pouch state was incorrect. I'm not exactly sure why because I didn't save the logs or footage of it happening to slow it down and see the plugin state slow enough to see actions/values faster than ticks.

I'll keep testing this PR for another hour or so to see what happens. However, even in the current state, I am confident that it is more accurate and useful than the existing Runecrafting plugins that are related to pouch tracking. That's why I'm still expecting to push these changes even if the issue persists albeit not often. It would be good to also implement #14 soon too.

I also am planning on adding a feature that will highlight pouches to empty according to inventory free space. I've already handled checking to see if the player is crafting runes so it should be relatively easy to do so too. A benefit of adding this feature would be that it would hopefully mitigate attempting to RC too quickly and causing wrong states.

Infinitay commented 1 week ago

TODO Refactor the code since there's lots of reused code I'm noticing after this PR primarily for removing (emptying) essence but also a few for adding essence. Currently, I'm planning to refactor it on another PR so I'll probably leave the PR as is.

Infinitay commented 1 week ago

Rebased branch onto main which brought in the degraded pouch fix from #18

Infinitay commented 1 week ago

Again, I couldn't actively recreate the issue where emptying the pouch returned an incorrect state like before when runecrafting and trying to empty the pouches too quickly.

However, even in the current state, I am confident that it is more accurate and useful than the existing Runecrafting plugins that are related to pouch tracking.

Noticed that this tends to happen in GOTR more often than regular runecrafting because in GOTR you get extra items when crafting that are only available for the minigame. I believe the issue is my inventory update blocking logic. Potential solutions:

  1. Refactor inventory updating
  2. Keep it simple and allow inventory updates if GOTR items were added to the inventory
    • Perhaps it's time to consider a utility class for all is<>ValidItem methods that check items
Infinitay commented 1 week ago

After the latest commit, I tested the plugin at GOTR for over 30 minutes and didn't face any immediate issues. Even when dealing with odd inventory states such as not having enough space to empty to the pouch it would correctly have n remaining essence as it should. I want to test it more tomorrow at GOTR playing normally, and then I want to test it by trying to break it both at GOTR and regular RCing such as at Astrals.

Infinitay commented 1 week ago

50 minutes of GOTR and 10 minutes of astrals - I couldn't break the plugin. I'm going to try for a bit longer and accept the PR. Over 2 hours of GOTR and 30-45 minutes of Astrals