Firebot icon indicating copy to clipboard operation
Firebot copied to clipboard

Addressing #2336 to default quotes to streamer name if no @ is provided

Open Wrongtown opened this issue 1 year ago • 3 comments

Description of the Change

When !quote add is invoked this change checks for an @ in the originator arg of the command, and if it doesn't find one we assume that the quote should be attributed to the streamer and splices in @+channelData.displayName. If there is an @ then that string is used for attribution as normal.

This is my first PR here so please do let me know if there's anything you'd have preferred I do differently.

Example

!quote add I'm so disappointed in you.

The above would attribute the quote "I'm so disappointed in you." to whoever the streamer is.

Counter-example

!quote add @zunderscore Do I have to do everything around here?

The above would attribute the quote "Do I have to do everything around here?" to twitch user @zunderscore

Update after review comments

Applicable Issues

#2336

Testing

  • Verified that when no @ attribution is included in an invocation of the !quote add command that the streamer username is attributed.
  • Verified that when an @ attribution is included in an invocation of the !quote add command that the @ attribution is still assigned as expected.
  • Verified that other commands in the !quote family do not appear to have somehow been caught up in the blast radius of my change.

After review comments

  • Verified that !quote add can be invoked as usual if user has not opted in.
  • Verified that the default to streamer functionality for !quote add works as expected if user does opt in.

Screenshots

image

Wrongtown avatar Aug 27 '24 16:08 Wrongtown

i have a question

!quote add zunderscore Do I have to do everything around here?

what about this use case

CKY- avatar Aug 27 '24 16:08 CKY-

The only thing I think we should add to this is make it configurable by the user. This is because some folks may not always use the @ symbol when referencing someone to quote. Difficulty with that is there's no good way for us to programmatically determine if the first word is a username, nickname or otherwise. So making it a configurable option (defaulted to off so current behavior doesn't suddenly change on users) puts the nexus on the Streamer to decide how they prefer it functions.

Makes sense, will look into that. Thanks for the prompt review!

Also for future reference if you end up submitting more PRs, please point them at the v5 branch. The v5 branch represents our active development branch whereas master represents what is currently released to users. 👍🏻

Dangit, I knew I'd forgotten something 🤦‍♂️

Wrongtown avatar Aug 27 '24 16:08 Wrongtown

i have a question

!quote add zunderscore Do I have to do everything around here?

what about this use case

You're quite right, if they intend to provide a username but don't include the @ it would attribute the quote "zunderscore Do I have to do everything around here?" to the streamer.

So as requested above, I'll look into adding a setting to opt-in on this change. If the streamer does opt in, the above would still happen, of course.

Wrongtown avatar Aug 27 '24 16:08 Wrongtown

@ebiggz I foolishly forgot what repo I was working on with 00b6ee700d0b0730c62c8ef75a6c0377d5de0f5d but thankfully that doesn't appear to have actually introduced any code change 🤦‍♂️

Wrongtown avatar Sep 12 '24 14:09 Wrongtown

@Wrongtown No worries! LGTM. Thanks again for working on this 👍🏻

ebiggz avatar Sep 13 '24 02:09 ebiggz