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

Tweak Whisper Display #1161

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

PlasmaRaptor
Copy link
Contributor

Tweak Issue #1130 - Make whisper display darker and italic, mimicking Goon Station 1 as requested.

Description

Description.


TODO

  • Task
  • Completed Task

Media

Example Media Embed


Changelog

🆑

  • add: Added fun :D
  • tweak: Tweaked fun
  • fix: Fixed fun!
  • remove: Removed fun :(

@github-actions github-actions bot added Changes: Localization Changes any ftl files Changes: YML Changes any yml files labels Oct 28, 2024
@SimpleStation14 SimpleStation14 changed the title Tweak whisper display Tweak Whisper Display Oct 28, 2024
@Mnemotechnician
Copy link
Contributor

Mnemotechnician commented Oct 28, 2024

  1. This hardcodes the message color, as opposed to the general consensus of "language colors should be displayed in whispers". Instead of doing that, you should modify the code to dim the color.
  2. Please, follow the PR template format. Or at least remove the unnecessary parts of it.

@PlasmaRaptor
Copy link
Contributor Author

  1. This hardcodes the message color, as opposed to the general consensus of "language colors should be displayed in whispers". Instead of doing that, you should modify the code to dim the color.
  2. Please, follow the PR template format. Or at least remove the unnecessary parts of it.

Ok, I will try to do this and resubmit it.

@Mnemotechnician
Copy link
Contributor

You can edit this PR instead, no need to re-submit.

@PlasmaRaptor
Copy link
Contributor Author

I didn't know that.

@PlasmaRaptor PlasmaRaptor reopened this Oct 28, 2024
@Remuchi Remuchi added Priority: 4-Low Should be resolved at some point Size: 5-Very Small For especially small issues/PRs Status: Awaiting Changes Do not merge due to requested changes Type: Feature Creation of or significant changes to a feature labels Oct 29, 2024
@PlasmaRaptor
Copy link
Contributor Author

I have edited it.

*It changes the text style to italic, making it much easier to notice.
*It keeps the color text for language, so you can easily identify the language.

This is not exactly what he requested but it is a big step in the right direction and hopefully will make his whispering experience pleasant. I could not dim the color after trying for a few hours.

@Aidenkrz
Copy link
Contributor

Aidenkrz commented Nov 7, 2024

Was this actually tested? I don't believe [italic] would work as italicized fonts are seperate fonts, no?

@Mnemotechnician
Copy link
Contributor

Mnemotechnician commented Nov 7, 2024

[italic] will override the [font] tag - so yes, it will KINDA work, but not as intended.

I wish italic fonts worked via a shader or something instead of being hardcoded, but they don't.

However, if we ever get language markers in any way, whether it's the way they are implemented in #1165, or some other way, that will become irrelevant. Using different fonts and colors based on the language was a questionable idea per se.

@PlasmaRaptor
Copy link
Contributor Author

I consider it a feature that will differentiate it even more, but if someone feels like editing it by dimming it and keeping the font the same etc. they're welcome to do it. This is just what I can do to accommodate his request to make whispering more distinguishable.

@Mnemotechnician
Copy link
Contributor

Also, there is now have two [italic] tags nested inside one another and a [font] tag that doesn't do anything because it gets overridden.

@PlasmaRaptor
Copy link
Contributor Author

Also, there is now have two [italic] tags nested inside one another and a [font] tag that doesn't do anything because it gets overridden.

I will quickly remove the extra one.

Removes redundant italics.
@github-actions github-actions bot added the Status: Merge Conflict FIX YOUR PR AAAGH label Nov 8, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@FoxxoTrystan FoxxoTrystan requested review from a team and VMSolidus and removed request for a team November 14, 2024 18:16
@github-actions github-actions bot added Status: Needs Review Someone please review this and removed Status: Awaiting Changes Do not merge due to requested changes labels Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: Localization Changes any ftl files Changes: YML Changes any yml files Priority: 4-Low Should be resolved at some point Size: 5-Very Small For especially small issues/PRs Status: Merge Conflict FIX YOUR PR AAAGH Status: Needs Review Someone please review this Type: Feature Creation of or significant changes to a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants