Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

raidboss: replace GameLog lines (HW) #95

Merged
merged 1 commit into from
Mar 14, 2024
Merged

raidboss: replace GameLog lines (HW) #95

merged 1 commit into from
Mar 14, 2024

Conversation

wexxlee
Copy link
Collaborator

@wexxlee wexxlee commented Mar 13, 2024

Some comments in-line. @JLGarber - tagging you since you did the initial Gubal Hard timeline. I really wanted to get rid of those GameLog syncs, so I took a look back at the discussion you had on the original PR, and I think this is the right implementation of the alternative (at least, it works, so 🤷 ).

@wexxlee wexxlee requested a review from JLGarber March 13, 2024 06:17
@github-actions github-actions bot added raidboss /ui/raidboss module needs-review Awaiting review labels Mar 13, 2024
Copy link
Collaborator

@valarnin valarnin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A11S and Dun Scaith changes look good to me. Can't really test the PvP changes, and don't have time right this moment to test the Gubal Library changes.

Some general thoughts in comments below, but they're not blockers for this PR to be merged, more of a follow up thoughts/notes thing.

Comment on lines +346 to 353
// There is a GameLog message (en: The plasma shield is shattered!), but no corresponding
// SystemLogMessage. The 0x19 (NetworkDeath) line shows up >2 seconds later (too late).
{
id: 'A11S Plasma Shield Shattered',
type: 'GameLog',
netRegex: { line: 'The plasma shield is shattered.*?', capture: false },

type: 'NetworkEffectResult',
netRegex: { name: 'Plasma Shield', currentHp: '0', capture: false },
response: Responses.spread(),
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not really directly related to this PR, but...

Relevant log lines that could potentially be used instead. 4096 lines are from my custom packet decoder.

Maybe we add SetStatus and/or DeathAnimation to ActorControlExtra in the future?

21|2024-03-13T15:26:49.9850000-04:00|10001234|Some One|25|Maim|40007E61|Plasma Shield|32710003|66540000|A3E|340000|11B|258000|0|0|0|0|0|0|0|0|0|0|16580|43607|8840|10000|||-3.47|49.52|-80.00|-2.12|424839|424839|10000|10000|||-6.37|47.48|-80.00|1.05|00002ADF|0|1|dcc2f14481805f17
4096|2024-03-13T15:26:49.9850000-04:00|ActionEffect1|10001234|40007E61|00000000|00000025|00002ADF|0.6|E0000000|814|0.958258623432312|0025|00|Spell|00|1|Damage|0|71|32|0|0|26196|False|False|Slashing|Unaspected|26196|Resource|A|0|0|0|0|52|False|False|Unknown|Unknown|52|StartActionCombo|1|0|0|0|80|37|False|True|Unknown|Unknown|37|Nothing|0|0|0|0|0|0|False|False|Unknown|Unknown|0|Nothing|0|0|0|0|0|0|False|False|Unknown|Unknown|0|Nothing|0|0|0|0|0|0|False|False|Unknown|Unknown|0|Nothing|0|0|0|0|0|0|False|False|Unknown|Unknown|0|Nothing|0|0|0|0|0|0|False|False|Unknown|Unknown|0|40007E61|00000000|e663350ba1169de3
4096|2024-03-13T15:26:50.6070000-04:00|ActorControl|40007E61|DeathAnimation|00000000|00000000|00000000|00000000|00000000|838eb2f80bc45df1
4096|2024-03-13T15:26:50.6070000-04:00|ActorControl|40007E61|SetStatus|00000002|00000000|00000000|00000000|00000000|07342aab0dbb92f2
4096|2024-03-13T15:26:50.6070000-04:00|ActorControl|40007E09|UnkVisControl|00000000|00000002|40007E61|00000001|00000000|c568b6550c899969
37|2024-03-13T15:26:50.6070000-04:00|40007E61|Plasma Shield|00002ADF|0|43607|0|10000|0||-3.47|49.52|-80.00|-2.12|0|0|0|01|0|0|0|E0000000|204bdbe24a735f12
4096|2024-03-13T15:26:50.6070000-04:00|EffectResult1|00000001|10975|40007E61|0|43607|0|00000000|0|1|00|0|00|0000|0000|0000|0.0000|E0000000|f493d8ae5ccb87f3
261|2024-03-13T15:26:50.2740000-04:00|Change|40007E61|CurrentMP|0|fce54aa4fea146a4
270|2024-03-13T15:26:50.6070000-04:00|40007E61|-2.1199|0000|003C|-3.4791|49.5315|-79.9890|9a83358620320738
4096|2024-03-13T15:26:50.6070000-04:00|ActorMove|40007E61|-2.1199|003C0000|-3.4791|-79.9890|49.5315|00000000|ec0648802d322489
38|2024-03-13T15:26:50.6070000-04:00|40007E61|Plasma Shield|003C3C00|0|43607|0|8840|0||-3.47|49.52|-80.00|-2.12|0|0|0|a5d618f4aa2f5bbb
4096|2024-03-13T15:26:50.6070000-04:00|StatusEffectList|40007E61|00|3C|3C|00|0|43607|0|8840|0|0000|00|0000|0000|0.0000|E0000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|0000|0000|0.0000|00000000|f0ddc0a471f9bb8b

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't wild about using the 0x25 line (even less so about using the 0x26 line), but having another option long-term would be nice. I'm not sure if DeathAnimation would have much use outside of this particular scenario, but I'm curious about SetStatus and if there might be broader use cases. Do you know any more about what effect that category of ActorControl does? Or whether there's a link between the first param and any of the exds?

Comment on lines -455 to +461
// Ordinarily we wouldn't use a game log line for this.
// Ordinarily we wouldn't use a log message for this.
// However, the RP text seems to be the only indicator.
// (Not a MapEffect packet either.)
// https://xivapi.com/LogMessage/2747
// en: Shadows gather on the floor.
id: 'Dun Scaith Shadow Links',
type: 'GameLog',
netRegex: {
line: 'Shadows gather on the floor.*?',
code: Util.gameLogCodes.message,
capture: false,
},
type: 'SystemLogMessage',
netRegex: { id: 'ABB', capture: false },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely unrelated to this PR but if we didn't want to use a SystemLogMessage here the only other option is the floor animation which is done via ActorControl:

41|2024-03-13T14:21:40.4020000-04:00|8003755F|ABB|00|00|00|759f799c80f38925
4096|2024-03-13T14:21:40.4020000-04:00|ActorControl|40000C88|EObjAnimation|00000004|00000008|00000000|00000000|00000000|ca4311e0e521125d
EObjAnimation = 413,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. How "spammy" of a category is EObjAnimation? Seems like another potential candidate for adding to OP.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OverlayPlugin/OverlayPlugin#276 (comment)

Guessing based on volume that the time period for those totals encompasses multiple days/log files (sorry if that's mentioned somewhere in the comment thread and I missed it). But 795 for EObjAnimation doesn't seem super heavy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that was over like a month's worth of logs, but it was also mainly level 90 roulette content or UWU/TEA IIRC.

@github-actions github-actions bot removed the needs-review Awaiting review label Mar 13, 2024
Copy link
Collaborator

@JLGarber JLGarber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran Gubal hard quickly and it seemed fine. The intention of what I was doing was preserved, although I kinda wonder whether it might have been better to just do forward-looking syncs like I did for Fractal Hard. Regardless, this works.

Given that valarnin already said the rest of the files are good, I'm comfortable calling this ready.

@wexxlee wexxlee merged commit 448ecf8 into OverlayPlugin:main Mar 14, 2024
17 checks passed
@wexxlee wexxlee deleted the gamelog-hw branch March 14, 2024 04:08
github-actions bot pushed a commit that referenced this pull request Mar 14, 2024
Some comments in-line. @JLGarber - tagging you since you did the initial
Gubal Hard timeline. I really wanted to get rid of those `GameLog`
syncs, so I took a look back at the [discussion you had on the original
PR](quisquous#693 (comment)),
and I think this is the right implementation of the alternative (at
least, it works, so 🤷 ). 448ecf8
github-actions bot pushed a commit that referenced this pull request Mar 14, 2024
Some comments in-line. @JLGarber - tagging you since you did the initial
Gubal Hard timeline. I really wanted to get rid of those `GameLog`
syncs, so I took a look back at the [discussion you had on the original
PR](quisquous#693 (comment)),
and I think this is the right implementation of the alternative (at
least, it works, so 🤷 ). 448ecf8
github-actions bot pushed a commit to ShadyWhite/cactbot that referenced this pull request Mar 17, 2024
Some comments in-line. @JLGarber - tagging you since you did the initial
Gubal Hard timeline. I really wanted to get rid of those `GameLog`
syncs, so I took a look back at the [discussion you had on the original
PR](quisquous#693 (comment)),
and I think this is the right implementation of the alternative (at
least, it works, so 🤷 ). 448ecf8
github-actions bot pushed a commit to ShadyWhite/cactbot that referenced this pull request Mar 17, 2024
Some comments in-line. @JLGarber - tagging you since you did the initial
Gubal Hard timeline. I really wanted to get rid of those `GameLog`
syncs, so I took a look back at the [discussion you had on the original
PR](quisquous#693 (comment)),
and I think this is the right implementation of the alternative (at
least, it works, so 🤷 ). 448ecf8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
raidboss /ui/raidboss module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants