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/oopsy: Aloalo variant left path #5941

Merged
merged 3 commits into from
Nov 22, 2023
Merged

raidboss/oopsy: Aloalo variant left path #5941

merged 3 commits into from
Nov 22, 2023

Conversation

quisquous
Copy link
Owner

No description provided.

Copy link
Owner Author

@quisquous quisquous left a comment

Choose a reason for hiding this comment

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

cc @kshman who wrote most of these triggers

Sorry if I have changed to much and have been too opinionated here. I think having the timeline makes the need for some of these triggers go away, and I'm trying to be consistent with what other triggers have done in the past.

Also I still haven't seen this ability ever: #5893 (comment). Where does this happen? Edit: oh this is probably the right path, sorry

Comment on lines -54 to -56
if (data.quaArmamentsCount === 3)
return output.third!();
return output.text!();
Copy link
Owner Author

Choose a reason for hiding this comment

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

I think the logic here was wrong. The third (and every one after) are all the donuts+axes combination.

alertText: (_data, _matches, output) => output.text!(),
outputStrings: {
text: {
en: 'Go to safe NOW!',
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is very subjective, but in playing through I think the timeline covers the timing this trigger is warning about better than an alert text.

infoText: (_data, _matches, output) => output.text!(),
outputStrings: {
text: {
en: 'Adds',
Copy link
Owner Author

Choose a reason for hiding this comment

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

Usually "Adds" is when there are adds to kill. I think in this case, there's obvious ground markers to avoid, and so there's no need to tell players about adds.

infoText: (_data, _matches, output) => output.text!(),
outputStrings: {
text: {
en: 'Adds',
Copy link
Owner Author

Choose a reason for hiding this comment

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

Same thing here re: Adds.

outputStrings: {
text: {
en: 'Avoid Chasing AOEs',
Copy link
Owner Author

Choose a reason for hiding this comment

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

I think the Chasing AOE puddles are not worth calling out here. There's no headmarker on players (so it's not targeted on somebody directly) and it's on the timeline too.

outputStrings: {
text: {
en: 'Bubbles float',
Copy link
Owner Author

Choose a reason for hiding this comment

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

I changed this trigger into separate triggers with more specific depending on the path you're on.

@kshman
Copy link
Contributor

kshman commented Nov 22, 2023

Sorry, I made a lot of things wrong. I'm very inexperienced. Very thank you! 😭

@quisquous
Copy link
Owner Author

Sorry, I made a lot of things wrong. I'm very inexperienced. Very thank you! 😭

Oh no, that's not what I was trying to say at all. I think the only thing that's even slightly wrong is just the Arcane Armaments 3 vs 4 count. The rest of it is just me having opinions about what's worth having a trigger about or what triggers are useful and isn't "right" or "wrong" in any way.

Thanks for all your PRs, they've been really appreciated!

@quisquous quisquous merged commit 6b91f8e into main Nov 22, 2023
6 checks passed
@quisquous quisquous deleted the aloalo_left branch November 22, 2023 16:57
kshman added a commit to kshman/cactbot-build that referenced this pull request Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants