Skip to content

Add 'Salvation' to notify's default messages#6959

Merged
lL1l1 merged 2 commits intoFAForever:developfrom
Gatsik:patch-1
Feb 16, 2026
Merged

Add 'Salvation' to notify's default messages#6959
lL1l1 merged 2 commits intoFAForever:developfrom
Gatsik:patch-1

Conversation

@Gatsik
Copy link
Contributor

@Gatsik Gatsik commented Nov 10, 2025

I noticed that there's no notify messages for 'Salvation' in the replay, but I'm not sure if this is all that's needed to add it

Summary by CodeRabbit

  • New Features

    • Added support for the experimental unit "Salvation" with display-name recognition and integration into Notify's default messages.
  • Documentation

    • Added a changelog entry noting inclusion of "Salvation" in Notify's default messages.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 10, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds a new experimental unit mapping for "Salvation" (xab2307) to the default notification messages and clarity display name; also adds a changelog snippet announcing the inclusion.

Changes

Cohort / File(s) Summary
Default messages
lua/ui/notify/defaultmessages.lua
Adds xab2307 = "Salvation" to defaultMessages.experimentals and clarityTable.
Changelog
changelog/snippets/fix.6959.md
Adds a changelog entry noting that "Salvation" is included in Notify's default messages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through tables in the night,
I added a name that gleams so bright,
Two keys tucked in just right,
Salvation now takes flight,
A rabbit's small code-fix delight.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete and missing most required template sections including testing details, documentation updates, changelog snippet, and reviewer assignment. Complete the description template by adding: testing verification, changelog snippet, reviewer assignment, and checklist items for documentation and comments.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding 'Salvation' to the notify system's default messages, which aligns with the file modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@lL1l1 lL1l1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works: salvation message appears in chat and the notify options menu.

@lL1l1 lL1l1 merged commit 00cb8dc into FAForever:develop Feb 16, 2026
5 of 6 checks passed
relent0r pushed a commit to relent0r/fa that referenced this pull request Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants