Conversation
📝 WalkthroughWalkthroughAdds several new UI tooltip entries, fixes/renames a public option key for reclaim batching distance, updates code that reads that option, and adds a changelog snippet documenting the tooltip additions. (50 words) Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lua/ui/help/tooltips.lua (1)
590-591: LOC key suffix convention is inconsistent with nearby entries.Other descriptive tooltip keys in this file use paired
_TITLE/_DESCRIPTIONsuffixes (e.g.,ASSIST_TO_UPGRADE_TITLE/ASSIST_TO_UPGRADE_DESCRIPTION,WATER_DEPTH_ASSISTANCE_TITLE/WATER_DEPTH_ASSISTANCE_DESCRIPTION). The new entry uses no suffix on the title key (INVERT_MOUSE_PAN) and an abbreviated_DESCsuffix on the description key (INVERT_MOUSE_PAN_DESC). This matters most if localization strings are ever added to a.locfile, as translators and tooling will expect consistent naming.♻️ Suggested rename for consistency
- title = "<LOC INVERT_MOUSE_PAN>Invert pan direction with middle mouse button", - description = "<LOC INVERT_MOUSE_PAN_DESC>When enabled, dragging with the middle mouse button moves the camera in the opposite direction.", + title = "<LOC INVERT_MOUSE_PAN_TITLE>Invert pan direction with middle mouse button", + description = "<LOC INVERT_MOUSE_PAN_DESCRIPTION>When enabled, dragging with the middle mouse button moves the camera in the opposite direction.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/ui/help/tooltips.lua` around lines 590 - 591, The LOC key names for this tooltip are inconsistent: change the title key "INVERT_MOUSE_PAN" to "INVERT_MOUSE_PAN_TITLE" and change the description key "INVERT_MOUSE_PAN_DESC" to "INVERT_MOUSE_PAN_DESCRIPTION" so they match the surrounding *_TITLE / *_DESCRIPTION convention; update the two entries in the tooltip table (the title and description fields in tooltips.lua) and ensure any corresponding .loc or lookup usages reference the new keys (search for INVERT_MOUSE_PAN and INVERT_MOUSE_PAN_DESC to update callers).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lua/ui/help/tooltips.lua`:
- Around line 590-591: The LOC key names for this tooltip are inconsistent:
change the title key "INVERT_MOUSE_PAN" to "INVERT_MOUSE_PAN_TITLE" and change
the description key "INVERT_MOUSE_PAN_DESC" to "INVERT_MOUSE_PAN_DESCRIPTION" so
they match the surrounding *_TITLE / *_DESCRIPTION convention; update the two
entries in the tooltip table (the title and description fields in tooltips.lua)
and ensure any corresponding .loc or lookup usages reference the new keys
(search for INVERT_MOUSE_PAN and INVERT_MOUSE_PAN_DESC to update callers).
NoryGit
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lua/ui/help/tooltips.lua (1)
673-676: Minor capitalization inconsistency in title
"Assist to upgrade radars"(sentence case) versus the adjacent"Assist to Upgrade"(title case) inoptions_assist_to_upgrade. Aligning to title case would keep these related entries consistent.✏️ Suggested change
options_assist_to_upgrade_radar = { - title = "<LOC ASSIST_TO_UPGRADE_RADAR>Assist to upgrade radars", + title = "<LOC ASSIST_TO_UPGRADE_RADAR>Assist to Upgrade Radars",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/ui/help/tooltips.lua` around lines 673 - 676, The title string in options_assist_to_upgrade_radar uses sentence case ("Assist to upgrade radars") while the related option options_assist_to_upgrade uses title case; update the title value in options_assist_to_upgrade_radar to Title Case (e.g., "Assist to Upgrade Radars") so it matches the casing style of options_assist_to_upgrade and keeps UI entries consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/ui/help/tooltips.lua`:
- Around line 681-684: The title/description pair for options_show_player_names
is contradictory (title says "Show Player Names" but description says it hides
them); update the description to describe the effect when the option is enabled
to match the title and the pattern used by other entries (e.g.,
options_painting, options_assist_to_upgrade). In the options_show_player_names
table change the description string to something like "When enabled, player
names are shown locally in the game; this change is client-side only." ensuring
the title remains options_show_player_names.title and the explanatory text is in
options_show_player_names.description.
- Around line 609-612: Rename the misspelled Lua key
options_reclaim_batching_distance_treshold to
options_reclaim_batching_distance_threshold everywhere it appears; update the
entry in the tooltip table (the shown diff), the options table entry, and the
reclaim handling code that references the key so lookups match the correctly
spelled key. Ensure you update all three occurrences and run a quick search for
any other references to options_reclaim_batching_distance_treshold to replace
them with the corrected key.
---
Nitpick comments:
In `@lua/ui/help/tooltips.lua`:
- Around line 673-676: The title string in options_assist_to_upgrade_radar uses
sentence case ("Assist to upgrade radars") while the related option
options_assist_to_upgrade uses title case; update the title value in
options_assist_to_upgrade_radar to Title Case (e.g., "Assist to Upgrade Radars")
so it matches the casing style of options_assist_to_upgrade and keeps UI entries
consistent.
lua/ui/help/tooltips.lua
Outdated
| options_reclaim_batching_distance_treshold = { | ||
| title = "<LOC OPTIONS_RECLAIM_BATCHING_DISTANCE>Reclaim Batching Distance Threshold", | ||
| description = "<LOC OPTIONS_RECLAIM_BATCHING_DISTANCE_DESCRIPTION>Adjusts the distance threshold for batching reclaim commands. Higher values group more props together.", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=lua -i "reclaim_batching_distance" -A 5 -B 2Repository: FAForever/fa
Length of output: 3283
Fix spelling: options_reclaim_batching_distance_treshold → options_reclaim_batching_distance_threshold
The Lua key options_reclaim_batching_distance_treshold misspells "threshold" as "treshold". This typo is consistent across the codebase (lua/options/options.lua line 1077 and lua/ui/game/reclaim.lua line 379), so the tooltip lookup will succeed, but the user-facing title correctly spells it as "Threshold". All three occurrences should be corrected to match proper spelling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lua/ui/help/tooltips.lua` around lines 609 - 612, Rename the misspelled Lua
key options_reclaim_batching_distance_treshold to
options_reclaim_batching_distance_threshold everywhere it appears; update the
entry in the tooltip table (the shown diff), the options table entry, and the
reclaim handling code that references the key so lookups match the correctly
spelled key. Ensure you update all three occurrences and run a quick search for
any other references to options_reclaim_batching_distance_treshold to replace
them with the corrected key.
| options_show_player_names = { | ||
| title = "<LOC options_show_player_names_title>Show Player Names", | ||
| description = "<LOC options_show_player_names_description>Locally hides player names in the game. This change is client-side only.", | ||
| }, |
There was a problem hiding this comment.
Title and description are semantically contradictory
The title is "Show Player Names", which implies that enabling the option shows player names. However, the description says "Locally hides player names in the game." — the opposite behaviour. Every comparable option in this file describes what happens when the option is enabled (e.g., options_painting: "When enabled, you can send and receive brush strokes…"; options_assist_to_upgrade: "When enabled, mass extractors automatically…"). As written, a user enabling this option would be confused.
The description likely needs to be reworded to describe the enabled state, for example:
✏️ Suggested rewording
options_show_player_names = {
title = "<LOC options_show_player_names_title>Show Player Names",
- description = "<LOC options_show_player_names_description>Locally hides player names in the game. This change is client-side only.",
+ description = "<LOC options_show_player_names_description>When enabled, player names are shown in the game. This change is client-side only.",
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lua/ui/help/tooltips.lua` around lines 681 - 684, The title/description pair
for options_show_player_names is contradictory (title says "Show Player Names"
but description says it hides them); update the description to describe the
effect when the option is enabled to match the title and the pattern used by
other entries (e.g., options_painting, options_assist_to_upgrade). In the
options_show_player_names table change the description string to something like
"When enabled, player names are shown locally in the game; this change is
client-side only." ensuring the title remains options_show_player_names.title
and the explanatory text is in options_show_player_names.description.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/options/options.lua`:
- Line 1077: The renamed profile key causes existing profiles to lose their
value; update the read site (e.g. in reclaim.lua where
GetFromCurrentProfile('options.reclaim_batching_distance_threshold') is used) to
attempt the new key first and, if nil, fall back to the old misspelled key
'options.reclaim_batching_distance_treshold' and use that value (then continue
using the default 150 only if both are nil); this one-line migration preserves
existing user settings without changing profile storage and can be removed after
a full migration cycle.
In `@lua/ui/help/tooltips.lua`:
- Line 609: The tooltip key is misspelled as
ooptions_reclaim_batching_distance_threshold — rename it to
options_reclaim_batching_distance_threshold so the options UI can find and
display the tooltip; locate the entry in lua/ui/help/tooltips.lua (the table key
currently named ooptions_reclaim_batching_distance_threshold) and correct the
key to options_reclaim_batching_distance_threshold, then run a quick UI hover to
verify the tooltip appears.
---
Duplicate comments:
In `@lua/ui/help/tooltips.lua`:
- Around line 681-684: The description for the tooltip entry
options_show_player_names semantically contradicts its title — update the
description to describe what the option enables (e.g., that toggling it controls
whether player names are shown locally and that states are 'on' vs
'allies-only') instead of saying it "locally hides" names; also verify the
tooltip lookup logic that resolves keys (the code that constructs the lookup
key, likely where Tooltips[...] is accessed) because the option key in
options.lua is already "options_show_player_names" and a lookup implementation
that prefixes "options_" would produce "options_options_show_player_names" (fix
the lookup to use the raw key or rename the tooltip entry to match the lookup).
Ensure you update the tooltip text for options_show_player_names and confirm/fix
the lookup code accordingly.
| { | ||
| title = "<LOC OPTIONS_RECLAIM_BATCHING_DISTANCE>Reclaim Batching Distance Threshold", | ||
| key = 'reclaim_batching_distance_treshold', | ||
| key = 'reclaim_batching_distance_threshold', |
There was a problem hiding this comment.
Silent reset of existing user profiles — consider migration fallback.
Renaming the stored key from reclaim_batching_distance_treshold to reclaim_batching_distance_threshold means any profile that already contains the old misspelled key will silently fall back to the default value (150) because GetFromCurrentProfile('options.reclaim_batching_distance_threshold') returns nil. No migration path is provided.
🛡️ Optional: one-liner migration in the threshold read site
A cheap fallback in reclaim.lua preserves old values without touching profile storage:
- if zoom < (Prefs.GetFromCurrentProfile('options.reclaim_batching_distance_threshold') or ZoomThreshold) then
+ local _threshold = Prefs.GetFromCurrentProfile('options.reclaim_batching_distance_threshold')
+ or Prefs.GetFromCurrentProfile('options.reclaim_batching_distance_treshold')
+ or ZoomThreshold
+ if zoom < _threshold thenRemove the fallback once a full profile-migration pass is done (or after one release cycle).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lua/options/options.lua` at line 1077, The renamed profile key causes
existing profiles to lose their value; update the read site (e.g. in reclaim.lua
where GetFromCurrentProfile('options.reclaim_batching_distance_threshold') is
used) to attempt the new key first and, if nil, fall back to the old misspelled
key 'options.reclaim_batching_distance_treshold' and use that value (then
continue using the default 150 only if both are nil); this one-line migration
preserves existing user settings without changing profile storage and can be
removed after a full migration cycle.
| description = "<LOC OPTIONS_0318>The higher the value, the easier it becomes to select and target units or props. The value is in (screen) pixels. This threshold is specifically used when watching a replay, it makes it easier to hover over units.\r\n\r\nInteraction with units take precedence over props.\r\n\r\nWhen multiple entities are in range, the one nearest to the mouse location has precedence.", | ||
| }, | ||
|
|
||
| ooptions_reclaim_batching_distance_threshold = { |
There was a problem hiding this comment.
Typo in tooltip key: ooptions_… → options_… — tooltip will not be shown.
The entry key starts with oo instead of o. Every other option tooltip in this file uses the options_ prefix. The options UI will look up options_reclaim_batching_distance_threshold, which will not match ooptions_reclaim_batching_distance_threshold, so the tooltip remains absent and the hover warning persists — directly contradicting the PR's stated goal.
🐛 Proposed fix
- ooptions_reclaim_batching_distance_threshold = {
+ options_reclaim_batching_distance_threshold = {
title = "<LOC OPTIONS_RECLAIM_BATCHING_DISTANCE>Reclaim Batching Distance Threshold",
description = "<LOC OPTIONS_RECLAIM_BATCHING_DISTANCE_DESCRIPTION>Adjusts the distance threshold for batching reclaim commands. Higher values group more props together.",
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ooptions_reclaim_batching_distance_threshold = { | |
| options_reclaim_batching_distance_threshold = { | |
| title = "<LOC OPTIONS_RECLAIM_BATCHING_DISTANCE>Reclaim Batching Distance Threshold", | |
| description = "<LOC OPTIONS_RECLAIM_BATCHING_DISTANCE_DESCRIPTION>Adjusts the distance threshold for batching reclaim commands. Higher values group more props together.", | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lua/ui/help/tooltips.lua` at line 609, The tooltip key is misspelled as
ooptions_reclaim_batching_distance_threshold — rename it to
options_reclaim_batching_distance_threshold so the options UI can find and
display the tooltip; locate the entry in lua/ui/help/tooltips.lua (the table key
currently named ooptions_reclaim_batching_distance_threshold) and correct the
key to options_reclaim_batching_distance_threshold, then run a quick UI hover to
verify the tooltip appears.
1 participant
Description of the proposed changes
This also removes the warnings when hovering over the affected options.
Checklist
- [ ] Changes are annotated, including comments where usefulSummary by CodeRabbit