Skip to content

Conversation

@harbingerftw
Copy link

@harbingerftw harbingerftw commented Sep 23, 2024

This is my first attempt at writing one of these so I'm sure there will be issues, but hopefully not too many. I am not sure if the way some callouts are created will work for localization but that can be re-worked.

There is still an issue I am working out with some rare-ish mechanic combos (sand spouts + orbs + tempests, usually if the fight goes really long) but wanted to go ahead and get feedback in case lots of things do have issues.

I also went ahead and made all the errors from testing go away, not sure if I was supposed to? I did lookup/copy the names of mobs in other languages.
I did talk to @wexxlee about working on this, and will hopefully do the other world boss faster than this.

@github-actions github-actions bot added raidboss /ui/raidboss module needs-review Awaiting review labels Sep 23, 2024
@wexxlee
Copy link
Collaborator

wexxlee commented Sep 24, 2024

Thanks so much for putting this together! I'm going to try to give this a run in the next few days (when my availability happens to coincide with the fate being up 😁) and see how it feels in game.

Copy link

@jacob-keller jacob-keller left a comment

Choose a reason for hiding this comment

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

Nothing looks too out of place here codewise. I'm not sure when I have a chance to test it out, and I didn't find a good existing log to test in the simulator.

@harbingerftw
Copy link
Author

I am going to be going back over more old logs/videos to double check, but this is complete I would say. I have elected to just not deal with the case where the boss rotates before a tempest (incomplete but still helpful callout).
Ttokrrone maybe got more effort than it deserved lol.
Can also share a log, not sure how you would prefer.

@wexxlee
Copy link
Collaborator

wexxlee commented Sep 26, 2024

(Just approved lint/test workflows to be run; haven't been able to test yet, but hoping to do so in the next day or two.)

@xiashtra
Copy link
Collaborator

You'll need to add 'missingTranslations': true, to the CN translation block.

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

@wexxlee wexxlee left a comment

Choose a reason for hiding this comment

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

Again, thanks so much for putting this together! This looked good to me in testing. A few nits below (mostly stylistic & some project-specific conventions).

I have elected to just not deal with the case where the boss rotates before a tempest (incomplete but still helpful callout).

I think that's fair, especially for an initial pass. (I got this combo in testing, incidentally). The callout was still useful enough. I think it would be worth noting this in a TODO at the top of the file, just so we have something besides PR comments to indicate that more could be done with it.

@github-actions github-actions bot added resources /resources needs-review Awaiting review labels Sep 28, 2024
@harbingerftw
Copy link
Author

I was going to add the in combat check to the triggers to match other world boss fates but since the triggers interact a lot, someone dying at the wrong time could cause issues so maybe I just leave them out?

I had also already looked up the translations but I forgot to push it seems.

@wexxlee
Copy link
Collaborator

wexxlee commented Sep 28, 2024

I was going to add the in combat check to the triggers to match other world boss fates but since the triggers interact a lot, someone dying at the wrong time could cause issues so maybe I just leave them out?

Yeah, given the cross-pollination between the triggers, I think leaving it commented is fine. Given these fates are not every-hour sort of things, I think it's better to have zone-wide triggers firing during the fate even if the player isn't in combat (e.g. flying in to the battle). If people happen to be in the zone, not in the fate, have cactbot turned on, and don't like the triggers, they have options for that... 😄

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

@wexxlee wexxlee left a comment

Choose a reason for hiding this comment

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

Changes generally look good to me! Just a few things to close on, and then I think this should be ready.

@github-actions github-actions bot added the needs-review Awaiting review label Oct 1, 2024
Copy link
Collaborator

@wexxlee wexxlee left a comment

Choose a reason for hiding this comment

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

Thanks again for all the work on this!

@github-actions github-actions bot removed the needs-review Awaiting review label Oct 1, 2024
@wexxlee wexxlee merged commit 40daa9e into OverlayPlugin:main Oct 1, 2024
github-actions bot pushed a commit that referenced this pull request Oct 1, 2024
This is my first attempt at writing one of these so I'm sure there will
be issues, but hopefully not too many. I am not sure if the way some
callouts are created will work for localization but that can be
re-worked.

There is still an issue I am working out with some rare-ish mechanic
combos (sand spouts + orbs + tempests, usually if the fight goes really
long) but wanted to go ahead and get feedback in case lots of things do
have issues.

I also went ahead and made all the errors from testing go away, not sure
if I was supposed to? I did lookup/copy the names of mobs in other
languages.
I did talk to @wexxlee about working on this, and will hopefully do the
other world boss faster than this. 40daa9e
github-actions bot pushed a commit that referenced this pull request Oct 1, 2024
This is my first attempt at writing one of these so I'm sure there will
be issues, but hopefully not too many. I am not sure if the way some
callouts are created will work for localization but that can be
re-worked.

There is still an issue I am working out with some rare-ish mechanic
combos (sand spouts + orbs + tempests, usually if the fight goes really
long) but wanted to go ahead and get feedback in case lots of things do
have issues.

I also went ahead and made all the errors from testing go away, not sure
if I was supposed to? I did lookup/copy the names of mobs in other
languages.
I did talk to @wexxlee about working on this, and will hopefully do the
other world boss faster than this. 40daa9e
@harbingerftw harbingerftw deleted the fate-ttokrrone branch October 9, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

raidboss /ui/raidboss module resources /resources

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants