-
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
Open
Jake2500
wants to merge
8
commits into
night:master
Choose a base branch
from
Jake2500:overlay
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Overlay Emote Modifier #6590
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
cc3d112
Add modifier to make an emote an overlay.
Jake2500 78231e3
Only allow overlay emotes over a non-overlay emote.
Jake2500 92514a3
Render the overlay modifier when incorrectly used.
Jake2500 d98a4cb
Render all invalid overlay modifiers
Jake2500 2a190f3
Merge remote-tracking branch 'origin/master' into overlay
Jake2500 c264be5
Lift constructing the replacement part out of the loop.
Jake2500 e4c13cc
Revert "Lift constructing the replacement part out of the loop."
Jake2500 372bdfc
Merge branch 'master' into overlay
dclstn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
stripping it works, but it also results in the modifier going missing from the output. for other modifiers, we do not show the modified emote but we show the modifier was sent when used incorrectly. is there a way to replicate this behavior here?
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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
I've modified it so that if an overlay modifier is removed, it replaces the corresponding parts in the part array.
I'm no artist, so feel free to use or not use this.
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.
@dclstn any ideas on the icon for this?
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.
Something like so perhaps:
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.
That's so much better 👏
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.
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
How's this?
downsized for reference:
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.
this looks better