Skip to content

Conversation

@Gatsik
Copy link
Contributor

@Gatsik Gatsik commented Dec 20, 2025

Fallback OptionShowPlayerNames to 'on' when it is nil, because game crashes when there are no options in game.prefs file
Most of the time (always?) GetFromCurrentProfile is checked for nil, but here this safety check was forgotten

Summary by CodeRabbit

  • Bug Fixes
    • Fixed player names in the armies table to display by default when user profile settings are unavailable, ensuring consistent visibility of player information.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 20, 2025

Walkthrough

The change adds a fallback default value for OptionShowPlayerNames in the armies table UI, defaulting to 'on' when the profile option is nil. This ensures the variable always contains a valid value rather than remaining undefined.

Changes

Cohort / File(s) Summary
UI player name option fallback
lua/ui/override/ArmiesTable.lua
Adds a nil-check with fallback default ('on') for OptionShowPlayerNames when retrieving from user profile, ensuring the option always has a valid value

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

  • Verify the fallback default value ('on') aligns with intended user experience
  • Confirm the nil-check logic doesn't inadvertently mask other profile issues

Poem

🐰 A default comes to stay,
When nil would lead astray,
Now 'on' shall light the day,
Names display, hip-hooray! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the reason for the change (game crash) and context (missing safety check), but lacks testing details and changelog documentation required by the template. Add a 'Testing done' section describing how the fix was tested and include a changelog snippet following the repository guidelines.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: adding a fallback default for OptionShowPlayerNames to 'on' when nil.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4dd9fdb and e9415a7.

📒 Files selected for processing (1)
  • lua/ui/override/ArmiesTable.lua (1 hunks)
🔇 Additional comments (1)
lua/ui/override/ArmiesTable.lua (1)

42-43: LGTM! The nil fallback prevents the crash.

The or 'on' fallback correctly handles the case when preferences return nil, defaulting to showing player names. This is a sensible default and the fix aligns with the defensive pattern used in the PR.

Both lua/ui/override/ArmiesTable.lua (line 43) and lua/ui/override/SessionClients.lua (line 36) have been updated with identical nil checks, ensuring consistent behavior across both player name display modules.

One observation: OptionShowPlayerNames is read once at module initialization and never re-read, even though caches may be updated at runtime. If the user's preference changes during gameplay, it won't be reflected until the module reloads. This appears to be existing behavior, not introduced by this change.


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.

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