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

Message highlights #2990

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

Toby222
Copy link
Contributor

@Toby222 Toby222 commented Feb 16, 2025

About the PR

Ported message highlighting from impstation/imp-station-14#468 and space-wizards/space-station-14#31442

Why / Balance

ADHD :)
Chat is hard to follow a bunch of the time, and finding relevant information at a glance is hard

Technical details

For auto-highlighting, uses your job's name for fetching what terms to highlight
Auto-highlighted strings are not persistent and only depend on your character

Media

example

Requirements

  • I have tested all added content and changes.
  • I have added media to this PR or it does not require an ingame showcase.

Breaking changes

Adds three new Client CVars

Changelog
🆑

  • add: Message highlighting

formlessnameless and others added 3 commits February 16, 2025 15:49
Add text highlighting

(cherry picked from commit e442cc4bb20fbb4d5b970275c82cc87508e6f687)
Edge case that's not handled: Closing and reopening the game drops the list of auto-generated highlights
@Toby222 Toby222 requested review from a team as code owners February 16, 2025 17:34
@github-actions github-actions bot added S: Needs Review size/L 256-1023 lines Changes: YML Changes any yml files Changes: UI Changes: C# Changes any cs files Changes: Localization Changes any ftl files labels Feb 16, 2025
@Toby222 Toby222 marked this pull request as draft February 16, 2025 17:41
Since the implementation is now fundamentally incompatible
with the (still open) upstream PR, move to our own cvars
@Toby222 Toby222 marked this pull request as ready for review February 16, 2025 18:11
@Lyndomen
Copy link
Contributor

Holy peak

@github-actions github-actions bot added size/XL Over 1024 lines and removed size/L 256-1023 lines labels Feb 18, 2025
deltanedas
deltanedas previously approved these changes Feb 18, 2025
@deltanedas deltanedas enabled auto-merge (squash) February 18, 2025 18:04
@Toby222

This comment was marked as outdated.

@Toby222

This comment was marked as outdated.

@Toby222
Copy link
Contributor Author

Toby222 commented Feb 18, 2025

If I'm already touching this with more fixes again
Should I be namespacing the new CVars?
Otherwise Imp/other implementations will overwrite ours

auto-merge was automatically disabled February 18, 2025 22:30

Head branch was pushed to by a user without write access

@Toby222
Copy link
Contributor Author

Toby222 commented Feb 18, 2025

Okay so I can't figure out why exactly or what to do about it, but the error is caused by RequestCharacterInfo being called before the entity gets its MetaDataComponent somehow.
Adding a if (uid.IsValid()) check before calling _characterInfo.RequestCharacterInfo() does not help.
I presume PlayerManager.LocalPlayerAttached is simply not the right event to listen to, but I'm not familiar enough with the codebase to know what else to use, and it's currently too late for me to keep looking

@Smugman Smugman mentioned this pull request Feb 19, 2025
2 tasks
@Toby222
Copy link
Contributor Author

Toby222 commented Feb 20, 2025

Fuck

@Toby222 Toby222 marked this pull request as draft February 20, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: C# Changes any cs files Changes: Localization Changes any ftl files Changes: UI Changes: YML Changes any yml files S: Needs Review size/XL Over 1024 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants