Skip to content

Conversation

@Legends0
Copy link
Collaborator

@Legends0 Legends0 commented Dec 20, 2025

cactbot tests response's output strings by executing the trigger with fake data which causes the console to log an error.

Fixes #852

cactbot tests response's output strings by executing the trigger with fake data which causes the console to log an error.
@Legends0 Legends0 changed the title raidboss: in trigger fix for #852 raidboss: marble dragon console log in trigger fix for #852 Dec 20, 2025
@github-actions github-actions bot added raidboss /ui/raidboss module needs-review Awaiting review labels Dec 20, 2025
@Legends0
Copy link
Collaborator Author

Ah, I missed the console log in pasting

@Legends0 Legends0 marked this pull request as draft December 20, 2025 16:49
@Legends0 Legends0 marked this pull request as ready for review December 20, 2025 16:51
@Legends0 Legends0 requested a review from xiashtra December 20, 2025 17:01
@Souma-Sumire
Copy link
Collaborator

I think that if the goal is just to eliminate the warning, changing console.error to throw new Error might be more straightforward?

@Legends0
Copy link
Collaborator Author

Legends0 commented Dec 20, 2025

I think that if the goal is just to eliminate the warning, changing console.error to throw new Error might be more straightforward?

I was keeping the log line for debugging if the trigger did not output. The return would be sufficient if a log isn't needed. I'm not sure if throwing an error would produce the log line since it doesn't in the config generation. Additionally, the amount of testing I've seen in game so far and it's a matches line probably indicates that logging of the position if invalid is no longer needed.

@Souma-Sumire
Copy link
Collaborator

image

Throwing an error will generate a log.

Removing it is also fine =v=

@valarnin
Copy link
Collaborator

It's important to note that popup-text catches the error in OnTrigger, which will not execute other parts of the trigger. It's not a problem with this trigger specifically, as it doesn't have e.g. a run function that needs to do data cleanup or anything, but it's also not a proper catch-all solution.

We also use console.(log|warn|error) in 283 locations across raidboss triggers currently.

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.

I just saw this while executing tests yesterday too. Thanks for fixing it.

Copy link
Collaborator

@xiashtra xiashtra left a comment

Choose a reason for hiding this comment

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

LGTM. I wonder if we should make a note in the documentation somewhere to avoid console.(log|warn|error) in response blocks, and also valarnin's comment about throwing possibly interrupting data cleanup in a run block.

@github-actions github-actions bot removed the needs-review Awaiting review label Dec 21, 2025
@Legends0 Legends0 merged commit a500456 into main Dec 21, 2025
12 checks passed
@Legends0 Legends0 deleted the mable-dragon-console-message-fix branch December 21, 2025 19:57
github-actions bot pushed a commit that referenced this pull request Dec 21, 2025
…#852 (#856)

cactbot tests response's output strings by executing the trigger with
fake data which causes the console to log an error.

Fixes #852 a500456
github-actions bot pushed a commit that referenced this pull request Dec 21, 2025
…#852 (#856)

cactbot tests response's output strings by executing the trigger with
fake data which causes the console to log an error.

Fixes #852 a500456
github-actions bot pushed a commit to ShadyWhite/cactbot that referenced this pull request Dec 21, 2025
…OverlayPlugin#852 (OverlayPlugin#856)

cactbot tests response's output strings by executing the trigger with
fake data which causes the console to log an error.

Fixes OverlayPlugin#852 a500456
github-actions bot pushed a commit to ShadyWhite/cactbot that referenced this pull request Dec 21, 2025
…OverlayPlugin#852 (OverlayPlugin#856)

cactbot tests response's output strings by executing the trigger with
fake data which causes the console to log an error.

Fixes OverlayPlugin#852 a500456
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.

Spamming of Log Messages

6 participants