Conversation
WalkthroughThe changes consolidate time formatting logic in the renderer by replacing templated time placeholders with a single runtime function. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/boss/render.cpp (1)
95-96: Consider extracting the repeated format-and-replace pattern.The pattern of
fmt::vformat()followed byutil::replaceAll(..., "$igt", format_time())is repeated four times. While not critical, extracting this into a helper method would reduce duplication.Possible refactor
Add a private helper method in the class:
std::string format_with_time(const std::string& template_text) const { auto text = fmt::vformat(template_text, args_); util::replaceAll(text, std::string("$igt"), format_time()); return text; }Then simplify the call sites:
// Instead of: auto text = fmt::vformat(challengeText_, args_); util::replaceAll(text, std::string("$igt"), format_time()); // Use: auto text = format_with_time(challengeText_);Also applies to: 99-100, 120-121, 124-125
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/boss/render.cpp(4 hunks)src/boss/render.hpp(2 hunks)
🔇 Additional comments (4)
src/boss/render.hpp (2)
24-24: LGTM! Clean method declaration.The new
format_time()method declaration is well-placed and appropriately marked asconst.
37-38: Good consolidation of format strings.Removing the hour-specific variants (
killTextHour_andchallengeTextHour_) in favor of a single dynamic format is cleaner and eliminates duplication.src/boss/render.cpp (2)
30-35: LGTM! Smart placeholder approach.Replacing
{igt}with$igtprevents fmt from attempting to format it as a chrono type, allowing for manual post-processing with the correctly formatted time string. This effectively sidesteps the modulo-24 limitation of%H.
193-205: Excellent fix! This correctly resolves the 24-hour rounding issue.The manual calculation of hours without applying modulo 24 is the key improvement. Previously, the
%Hchrono format specifier would wrap at 24 hours (e.g., 208 hours → 16:00:00), but this implementation correctly displays the full hour count (208:00:00).The logic is sound:
- Hours:
total_seconds / 3600(no modulo)- Minutes:
(total_seconds / 60) % 60- Seconds:
total_seconds % 60The conditional format (showing hours only when > 0) is also a nice touch for readability.
Fixed issue when igt was rounding to 24h because of the
"{igt:.0%H:%M:%S}"patternFixed IGT vs original IGT % 24 (208 % 24 = 16)

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