-
Notifications
You must be signed in to change notification settings - Fork 267
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
Overlay Emote Modifier #6590
base: master
Are you sure you want to change the base?
Overlay Emote Modifier #6590
Changes from 2 commits
cc3d112
78231e3
92514a3
d98a4cb
2a190f3
c264be5
e4c13cc
372bdfc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ const EMOTE_MODIFIERS = { | |
'c!': 'bttv-emote-modifier-cursed', | ||
'l!': 'bttv-emote-modifier-rotate-left', | ||
'r!': 'bttv-emote-modifier-rotate-right', | ||
'o!': 'bttv-emote-modifier-overlay', | ||
ffzW: 'bttv-emote-modifier-wide', | ||
ffzX: 'bttv-emote-modifier-flip-horizontal', | ||
ffzY: 'bttv-emote-modifier-flip-vertical', | ||
|
@@ -288,6 +289,7 @@ class ChatModule { | |
|
||
const parts = data.split(' '); | ||
const partMetadata = []; | ||
const isOverlay = []; | ||
let hasModifiers = false; | ||
let modified = false; | ||
for (let j = 0; j < parts.length; j++) { | ||
|
@@ -325,6 +327,7 @@ class ChatModule { | |
let modifiers = []; | ||
if (hasModifiers && isEmoteOrSuffixModifier) { | ||
let detectedEmote = false; | ||
let predecessor = null; | ||
// we search backwards to find the emote and any modifiers | ||
for (let k = j; k >= 0; k--) { | ||
const partMetadataItem = partMetadata[k]; | ||
|
@@ -341,11 +344,35 @@ class ChatModule { | |
(!detectedEmote && partMetadataItem.type === 'prefix') || | ||
(detectedEmote && partMetadataItem.type === 'suffix') | ||
) { | ||
predecessor = k; | ||
break; | ||
} | ||
modifiers.push(partMetadataItem); | ||
parts[k] = null; | ||
} | ||
|
||
const overlayModifierPredicate = ({modifier}) => modifier === 'o!'; | ||
const negate = (pred) => (x) => !pred(x); | ||
|
||
// An emote may be an overlay if it satisfies the following conditions: | ||
// 1. It has a "predecessor" part. | ||
// 2. That predecessor part is an emote. | ||
// 3. That predecessor emote is not an overlay. | ||
// 4. The user has applied the overlay modifier. | ||
// This ensures that overlays do not stack and they only stack on another emote. | ||
if ( | ||
predecessor != null && | ||
partMetadata[predecessor]?.emote && | ||
!isOverlay[predecessor] && | ||
modifiers.some(overlayModifierPredicate) | ||
) { | ||
// Keep track of this part being an overlay. | ||
isOverlay[j] = true; | ||
} else { | ||
// Strip the overlay modifier from the part. | ||
modifiers = modifiers.filter(negate(overlayModifierPredicate)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as an aside, do you have an image asset for this modifier? if not, we can probably have one made There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dclstn any ideas on the icon for this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's so much better 👏 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the image is small in emote form, it kind of just looks like 1 square. Maybe make it so that the top square doesn't overlap the bottom one quite so completely? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed with Dallas' feedback. you can't tell what this is when smaller. it also resembles a copy button, which is not the intention. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks better |
||
} | ||
|
||
// if the emote is only a suffix modifier, render it without its effect | ||
if (modifiers.length === 1 && modifiers[0].type === 'suffix' && emoteIndex === j) { | ||
modifiers = []; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we pull these function definitions out of the hot path? they look re-usable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved these out and near the definitions of the modifiers.