external-plugins icon indicating copy to clipboard operation
external-plugins copied to clipboard

[PLANK SACK] Plank sack gets planks substracted when a "repair" action is queued while still building the previous feature in Mahogany homes

Open geeckon opened this issue 1 year ago • 1 comments

This issue is for the external plugin: PLANK SACK Plugin version: 1.3.1

Issue: While doing Mahogany Homes, if you click to "Repair" a feature while the animation for your previous build action is in progress, a plank will be substracted from your plank sack at the end of this animation. This happens more often than not, but not always.

Cause: It seems that the issue is caused by the "watchForAnimations" mechanic implemented in the plugin. This boolean is set as true whenever you click to "Repair" a feature. This leads to a possibility that a "watchForAnimation" is queued, but then triggered by the previous build that was being done. If the player keeps spamming to "Repair", "watchForAnimation" is set to true again and then triggered (correctly) by that build. https://github.com/Enriath/external-plugins/blob/5c460f8ffa6a1d7ec33e74f9c1efef73196ae104/src/main/java/io/hydrox/planksack/PlankSackPlugin.java#L364C19-L364C19 This doesn't always occur, which leads me to think there might be a race condition involved.

Video evidence: https://github.com/Enriath/external-plugins/assets/24608301/5301efae-3678-4d46-875d-de78d05eb701

Workaround: This issue has a workaround. If you walk to the feature that needs to be repaired and then click "Repair" or just wait for the previous build animation to finish before clicking "Repair", the issue does not occur.

Possible solutions: I can see 2 solutions to fix the issue, listed below from most to least appropriate in my oppinion:

  1. Scrap the "watchForAnimations" and instead implement a "watchForRepairXpDrop". Since the "Repair" builds have specific xp values (https://oldschool.runescape.wiki/w/Mahogany_Homes#Experience_costs), it is possible to identify when one of such builds is completed. Either xp drops or xp changes can be subscribed to (examples of both can be found in my instant damage calculator plugin https://github.com/geeckon/instant-damage-calculator), although tracking xp changes would probably be more reliable. XP boosts like the Carpenter's outfit, Leagues (and possibly others) would need to be taken into account. Actually the whole plugin (or at least the Mahogany Homes actions) could be rewritten to base everything on xp drops, but probably don't need to fix what isn't broken.

  2. Implement a "skipCurrentAnimation" which would indicate that the build animation that is currently in progress, should be skipped when a "watchForAnimations" has been set. This would be set to true when a build is started from a menu (https://github.com/Enriath/external-plugins/blob/5c460f8ffa6a1d7ec33e74f9c1efef73196ae104/src/main/java/io/hydrox/planksack/PlankSackPlugin.java#L274C4-L274C4), checked for and set to false when handling an animation (https://github.com/Enriath/external-plugins/blob/5c460f8ffa6a1d7ec33e74f9c1efef73196ae104/src/main/java/io/hydrox/planksack/PlankSackPlugin.java#L364C19-L364C19).

geeckon avatar Dec 29 '23 07:12 geeckon

Thanks for the looking into this! I tried debugging this issue in the past but couldn't figure out how to replicate this consistently.

I played around and was able to replicate this bug consistently without spam clicking.

Basically, after you click "build", the race condition is between you clicking "repair" and the next tick which starts the animation.

SeaifanAladdin avatar Mar 28 '24 02:03 SeaifanAladdin