false-spring / gbfr-logs

GBFR Logs lets you track damage statistics with a nice overlay DPS meter for Granblue Fantasy: Relink.
MIT License
214 stars 25 forks source link

Add special handling for Ferry #112

Closed RyanBuccellato closed 7 months ago

RyanBuccellato commented 7 months ago

Hello! Disclaimer: this is the first time I've written Rust code so I apologize in advance if I'm breaking conventions or it's sloppy for the language. Feel free to make any changes or take and leave what you deem appropriate, just laying some groundwork here.

This is a set of changes to fix damage attribution for Ferry's pets. Pretty much there is currently an issue where pet damage gets attributed to the wrong skills. This is because the action_id coming back for these skills gets updated to the last ability used. For example if you use the Strafe skill then dodge, the damage events that come after the dodge have the skill id for Dodge rather than strafe. While this is an issue with where the numbers are coming from I wasn't sure if it was even possible to fix given that information on the events comes from the game itself I believe?

Fortunately the actor ids let us know when the damage is coming from a pet. There is also a flag that seems to indicate the damage is coming from a pet skill rather than a pet auto attack, so that lets us attribute normal pet damage via seeing if that flag is off.

However for Skills the problem is not entirely fixable. The way I have this fix set up, it will attribute skill damage to the last pet skill seen. So if you start a strafe then dodge it will continue counting towards strafe until it sees a new pet skill. One issue here is if you dodge before any strafes hit the very first action_id is for dodge and there is never an action_id for Strafe... as far as I can tell there is no true way to tell what the ability was. In such cases we just drop damage into the standard "pets" bucket. It's also possible to misattribute one pet skill for another via a similar manner. While unfortunate it's at least better than it currently is. One longer term fix may be to just assign pet skill damage to it's own bucket. e.g. "Nicola - Skill". So even if the skill name is not there you can kind of guess that Nicola - Skill damage is from whatever skills were summoning him. It's not as descriptive but should be 100% accurate at least. The current approach is not 100% accurate if you cancel a lot but for most people probably makes more sense. Also pet skill damage will only incorrectly go into other pet skills, at the very least it should no longer leak into every ability you use.

Also sidenote: The pet damage portion of Onslaught will now go into the "Pets" bucket how I have it. It's easy to put it back in Onslaught if we want but since the damage cap is the same as the pet auto damage cap I think it actually makes more sense under Pets. It also doesn't benefit from supplementary damage like the rest of Onslaught so it's good to have it separate for mathing out how valuable supp damage would be for you. Ideally I think there would be another "Onslaught - Pet" bucket we could toss it in but I didn't want to make new buckets since then I would need to translate etc.

Screenshots

Pre-fix: ferryPreFixDps

You can see "Dodge (?)" has eaten up a lot of the Strafe damage. You can also see the pet attacks have no category and are eating into the other ones like Attack 1 (min damage is 18.4k which is the pet attack).

Post-fix: ferryPostFixDps

I did roughly the same set of actions for this one but you can see that the "Dodge (?)" is now gone and all the damage is attributed to Strafe. You can also see the Pet autos after now are properly in the "Pets" section and are not attributed to the Attack 1 at all (min values are as we expect)

RyanBuccellato commented 7 months ago

Important to note I do not think this fixes the online play portion of https://github.com/false-spring/gbfr-logs/issues/41

However it should clear up some of the confusion there about where the pet damage is going

RyanBuccellato commented 7 months ago

Thanks, sounds good given that the next patch is supposed to be in the next couple weeks waiting probably does make sense. Especially since I suspect this is actually related to an existing in-game bug atm that has gameplay implications so maybe they will fix it.

The current in-game bug is if you just Strafe and do nothing else her SBA stops generating at 2.5%. But if you keep dodging or doing other abilities the SBA will "unstick" and continue generating like normal and she can get as much as 7.5% SBA from the Strafe itself. This also happens for pet auto attacks (if you just let them go and stay still you stop generating SBA eventually, but dodging will cause them to start generating SBA again). This is only a guess but I suspect this has to do with the weird way the damage for her pets gets re-attributed to whatetever Ferry's last action was (otherwise why would dodging affect her pet attack SBA gain?).