Forces count seems to have issues with "count forces above 100%" feature

[happenslol]

@Aeon234 Hey, seems like the feature we merged did break something after all.

I've had bug reports that the forces are no longer counted, e.g.:

Since this happened right after the release and that's the exact part of the codebase that was touched, it's very probable that it was the cause. The old version is still up (2.8.0), but I've reverted the changes and pushed a patch version (2.8.1).

Before we re-merge anything, could you do some more testing, especially with users of the addon that weren't part of the development/testing before? I previously used to give out beta versions to my M+ team, but as I mentioned, since I'm not playing at the moment (and neither are they) I can't help with that.

[Aeon234]

I'll take a look and have my mplus team + guildies take it for a test drive. I even ran keys on 2.8.0 and everything was working fine. I'll report back with what I find

[Aeon234]

Just a status update.

Had both my own mythic plus team and my raid team test it out. combined about 10 ppl tested it and for all of them it worked without a problem. Be it showing forces up to 100% or over 100%.

I did find a small bug in some occasions so we fixed that up and now having everyone re-test to make sure that both it works and the bug is fixed. Will report back tomorrow after everyone runs some keys.

[jcamarao]

@happenslol

Hey, sorry for the long delay. Fixing the bug got put on a hold for a bit since I got busy with work and his m+ group had to pause while a member was moving. They're back to running keys now though.

However, we've finally finished fixing up the bug that was happening with counting. It'd always go over 100%, but in some instances it would calculate wrong. I figured out it was due to the add-on sometimes running OnCombatLogEvent first, other times running UpdateForces first. The logic I intially had wasn't robust enough to handle both cases since for some reason, currentCount would update internally even when I couldn't find where it was being called. It was also just a random occurrence. It could work fine for 10-15 keys straight, then one run would be off by a little. I changed it so regardless of the order, it should count correctly. @Aeon234 has been running keys for the past week and it hasn't failed or bugged yet. He's handed the new version to his m+ group and they're going to run keys over the next week or so. If it looks all good, we'll make another PR.

Also, neither Aeon nor his m+ group has ever once run into an instance of it not counting trash % entirely. Not sure what the issue was with the user in your screenshot.

[happenslol]

Seems I owe you an apology then. The timing of the comment I got was right when the new version was published, so the connection seemed obvious to me, but it must've been a fluke.

Thanks for testing it so thoroughly. I'm happy to merge the feature again if you open another PR!

[jcamarao]

@happenslol Hello! Sorry for the long delay again. I'm pretty sure it's working. Aeon has run keys for the past 3-4 weeks without any count going off. However, he hasn't run enough for me to want to make a PR. I'd wager only maybe in the ball park of 20ish keys (however he did give his m+ buddies the current version after 5 keys or so - so that's maybe 15 keys of them all using it and not failing for a single person). Still, I want him to do more before we actually make a PR rather than merge in something that has a bug.

Hopefully that'll happen in the next 2-3 weeks depending on whenever he runs more keys to test every scenario, but it looks good so far.

It hasn't failed yet when:

1. unclampforcespercent is enabled with MDT installed
2. unclampforcespercent is enabled without MDT installed
3. unclampforces disabled

Will make a PR whenever I feel it's tested robustly enough. Thanks for patience.

[happenslol]

Hey, what's the status on this? I've rewritten a lot of the counting logic with the changes introduced in TWW in 4.0, so the code for this will probably need to be ported. I'd be happy to help with that, but if no one is working on this anymore I'd like to close out the issue or at least mark it as a feature request that's open to contributions.

[jcamarao]

Sorry for the late response and silence! Aeon actually took a break from M+ before TWW launch, so we paused testing then. Then, I redid some of the logic of the count at the start of TWW to make it cleaner. Got it to a complete state after on/off testing a month or so back, and Aeon has been testing it/giving it to his guild to test as well. It was working fine for 3-4 weeks for everybody, then he ran into an instance where the count didn't tick for the very last mob earlier this week. So I've been investigating that, but have been getting busy with work.

Feel free to push up all of the new rewritten counting logic and I'll refactor it to fit the new code.

[jcamarao]

oh, I'm dumb. I saw you pushed it up already, haha. I'll work on refactoring it.