Fight summary side panel tracking per hit rather than per fight

[pwatts6060]

Related plugin component(s) Statistic: Fight Log side panel

Describe the bug Fights are not being tracked correctly anymore as of a recent update. They appear to be tracking 1 hit at a time rather than summarizing multiple hits. This appears like there are multiple separate fights with the same person with each being 1 hit. Click on the fight summary doesn't open the normal window either.

To reproduce Fight anyone in wilderness and see side panel

Expected behavior Should track many attacks in one summary entry

[evankroz]

+1 this, started happening about 8 hours ago.

[Matsyir]

Uh oh. Probably just turn off the plugin for the time being.

Very busy this whole summer + this coming weekend, I'll probably look into this earliest next Monday or Tuesday (Aug19/20). will gladly push a fix if it's some small changes, and I do happen to have membership for testing right now. No promises if it happens to be some complex issue.

If anyone could confirm that this issue also occurs in LMS, and not just the wilderness, that could save me some headaches. It's usually where I go to test the plugin easily. I'm assuming it's everywhere though, shouldn't be anything different between the two when it comes to that type of tracking - just asking because OP specified wildy.

Any other devs, feel free to have a look if you'd like to see it fixed sooner.

[evankroz]

Would you happen to know where the problem might be originating?

I havent really seen the code, and arent too experienced with Java, more Python/C stuff. I can try to see if it is some elementry problem that can be solved.

Ill look through some of the files, see if anything stands out

[nasifra]

If anyone could confirm that this issue also occurs in LMS, and not just the wilderness, that could save me some headaches. It's usually where I go to test the plugin easily. I'm assuming it's everywhere though, shouldn't be anything different between the two when it comes to that type of tracking - just asking because OP specified wildy.

Can confirm it's an issue in LMS as well. I just came to report this very bug after an LMS game.

I also noticed pre-update that the plugin recently seems to struggle to figure out who won/lost the fight. I have ambiguous LMS entries that don't show a skull by either name. I don't PK in the wildy so I can't say if it happens for kills/deaths there as well.

Edit: I mainly program in C# but I have some Java knowledge and some limited RuneLite API knowledge, so I could perhaps take a look tomorrow.

[evankroz]

Were there any updates that were somehow pushed out very recently?

This has never been a problem, and is extremely weird considering how well the plugin worked before today.

The issue persists in both LMS and Wildy.

Edit: Looks like ruenelite had a new release, 1.10.36, 14 hours ago, which is when the issue started occuring, slight off-chance: that the bug is because of the new update, although it is hard to tell considering the new release seemed to have changed nothing in terms of client plugins

[Matsyir]

Would you happen to know where the problem might be originating?

Well, it's definitely related to some kind of RL update, or some kind of change in how OSRS/Jagex transmits data to the client, considering there hasn't been any recent updates to the plugin related to this behavior, and it just randomly stopped working.. maybe something random like an equality check for a specific type isn't as straightforward as it used to be. Maybe player object references are now constantly randomized or something like that. No idea yet, just throwing out random ideas.

The related logic for this would most likely be found in PvpPerformanceTrackerPlugin.java, or some of the functions called within the following events in that file:

Fights get partially started in the onInteractingChanged event, when you click on a player, and/or when they click on you back, but they only "actually" start and get saved once both players have performed attacks on each-other via the onAnimationChanged event. Fights also end via onAnimationChanged, when a death animation is detected.

Fights often end too quickly since they end instantly when a death animation is detected, so double deaths can't be detected, unless they are sync'd. Maybe fixing that behavior could be a step towards fixing this bug - but I don't really think it's related, since it doesn't sound like death animations are any part of this issue.

At some point in the past there was previously some odd behaviors regarding properly persisting fights, as well as properly ignoring actions which weren't targeted to you (for example, there used to be a bug of something like, if you followed a player who attacked another player, their attacks would be tracked as against you)... due to that, there may already be some odd behaviors/fixes in place in regards to these events.

P.S: For any devs diving into this before I do, I'd strongly recommend you turn on dev tools while you do, and double check that some critical hardcoded values involved with this bug haven't changed (for example, the death animationID)

[Matsyir]

From @ alexsuperfly in the RL Discord #dev channel (a little bit of extra discussion/details followed this message in the channel):

i know at least this changed to 2048 https://github.com/Matsyir/pvp-performance-tracker/blob/9fb9a9a8630331a038248d7f76c9ca0e5f206569/src/main/java/matsyir/pvpperformancetracker/PvpPerformanceTrackerPlugin.java#L992-L995

I believe this'll be the primary culprit of this issue. Tons of the plugin's behavior is built on top of this itemID detection. Will try apply and test this fix on Monday, it's very possible that simply changing these two hardcoded values to 2048 will fix everything for new fights.

One tricky thing is that we'll have to migrate/update data if we want to continue supporting older fight data, from before this Wednesday. I'm pretty sure the saved json data, when it comes to itemIDs, are saving the "raw" itemID, before the -512 fix (not 100% sure on that, been a while, might've changed it at some point.. but like, 99% sure). There are already some systems in place for updating/migrating data, but it's maybe a little uh, esoteric, and involves conditionals on hardcoded values, so I wouldn't really expect another dev to implement that - feel free to try if you'd like though!

The best way to migrate the data would be to keep the "real ItemIDs" in the fight data, instead of the "raw" ones that need to "be fixed" (by doing -512 before Wednesday, now -2048). And then for new/ongoing fights, we should immediately apply the -2048 to all ItemIDs instead of saving the "raw itemID" as we have been previously.

An alternative, much easier way of fixing this while supporting old data, would be doing something like -2048 +512 on the new data, and continue using the old -512 behavior where saved data is used... but I really hate that - I'll try and implement the migration described above on Monday and finally use the "real ItemIDs" in the saved data.

---

UPDATE: Discord user @ peenay has quickly tested the fix of swapping the 512 offsets with 2048, and that fixes new fights going forward. I would like to try implementing the migration before implementing this hotfix, as having an "in-between version" during the data migration may complicate things and require use of dates, which may not work depending on how user's system clocks are setup.

I am not at all worried about fixing or removing these bugged fights with 1 attack dealt, just migrating and supporting older, real fights from before this Wednesday when this bug started occurring. Might add a config option for "exclude fights with under X total attacks", defaulted to 2 or 3, this seems like it should be helpful regardless of this bug - e.g for hiding those quick "fights" that occurred in multi clan fights (anyone who's done multi pvp with this plugin, I'm sure you know what I mean).

[tylerwgrass]

RuneLite now provides a const for the item offset you can use instead of hardcoded values: https://github.com/runelite/runelite/blob/db4e6d62eceaf1f1bb032cefaab7d572ab7de77d/runelite-api/src/main/java/net/runelite/api/PlayerComposition.java#L37

[evankroz]

Yep, this most likely was the problem, pretty interesting how I overlooked that when going through the latest runelite update.

Big thanks for spotting that!

[Matsyir]

I've just submitted the update for this, you can follow the progress of that being merged here: https://github.com/runelite/plugin-hub/pull/6490

I didn't manage to entirely test it tonight, but it should be good - I'll try to test it ASAP once it's merged.

Didn't get to update/migrate old fight data just yet, but the hotfix was put out in a way that it should be viable to update your old/existing fights in a future update when I have more time to do everything properly.

[Matsyir]

it got merged and its working yipeeee

Big thanks once again to discord user Pine / @ peenay for testing and providing the code for the core of the fix.

As I mentioned previously, old fights are gone for now but may be restored in the future (they're still saved, just not being loaded)