Feature: Add decimal numbers setting for solve time display#108
Feature: Add decimal numbers setting for solve time display#108
Conversation
- Add DECIMAL_NUMBERS key to SettingOptionKeys - Create DecimalNumbers enum with TWO (2) and THREE (3) options - Add decimalNumbers flow and setter to SettingsRepository and SettingsRepositoryImpl - Add decimalNumbers to SettingsState, SettingsAction, SettingsViewModel, LocalSettings - Add string resources for decimal_numbers in EN and ZH - Add SettingsDropdownTile for decimal numbers in SettingsScreen - Add toFormattedString(numOfDec) method to TimeResult - Update PlayerTimerComponent, StatsComponent, SolveCard to use decimalNumbers setting Co-authored-by: ricky9667 <55730003+ricky9667@users.noreply.github.com>
|
@copilot I noticed that when i change to 3 decimals, the 3rd decimal of the time data is always 0. Check |
Change round(2) to round(3) in TimeResult.finalTime so that millisecond precision is stored when the 3-decimal setting is active. Previously the 3rd decimal was always 0 because finalTime discarded it before toFormattedString was called. Co-authored-by: ricky9667 <55730003+ricky9667@users.noreply.github.com>
The root cause was in |
...oseApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/domain/options/DecimalNumbers.kt
Outdated
Show resolved
Hide resolved
- Remove DecimalNumbers.kt enum class - Change SettingsRepository, SettingsRepositoryImpl, SettingsState, SettingsAction, LocalSettings to use Int directly - Inline the decimal numbers dropdown in SettingsScreen showing "2"/"3" as plain text (no string resources needed) - Remove decimal_numbers_2/decimal_numbers_3 string resources from EN/ZH - Update all call sites: appSettings.decimalNumbers.value -> appSettings.decimalNumbers Co-authored-by: ricky9667 <55730003+ricky9667@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a configurable decimal precision setting for solve time displays across the HushTimer speedcubing app. The feature allows users to choose between 2 or 3 decimal places, and fixes a bug where the 3rd decimal was always shown as 0 due to premature rounding in the TimeResult calculation.
Changes:
- Introduced a new
decimal_numberssetting (default: 2, options: 2 or 3) stored as a plain Int throughout the settings stack - Refactored time formatting logic from extension functions to a centralized TimeFormatter utility object with improved rounding precision
- Updated all time display components (timer, statistics, session cards) to read and apply the user's decimal preference
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/utils/TimeFormatter.kt | New utility object centralizing time formatting logic with epsilon-corrected rounding |
| composeApp/src/commonTest/kotlin/com/rickyhu/hushtimer/utils/TimeFormatterTest.kt | Comprehensive test suite for TimeFormatter utility functions |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/core/domain/TimeResult.kt | Changed finalTime rounding from 2 to 3 decimals and added toFormattedString method with configurable precision |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/core/domain/Util.kt | Removed extension functions for time formatting (moved to TimeFormatter) |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/domain/SettingsRepository.kt | Added decimalNumbers flow and setter method |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/domain/SettingOptionKeys.kt | Added DECIMAL_NUMBERS constant |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/data/SettingsRepositoryImpl.kt | Implemented decimalNumbers flow and setter with default value of 2 |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/presentation/SettingsState.kt | Added decimalNumbers property |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/presentation/SettingsAction.kt | Added OnSelectDecimalNumbers action |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/presentation/SettingsViewModel.kt | Wired decimalNumbers into settings flow and action handler |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/presentation/LocalSettings.kt | Added decimalNumbers to AppSettingsProvider |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/settings/presentation/SettingsScreen.kt | Added inline dropdown UI for decimal numbers selection |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/timer/presentation/components/PlayerTimerComponent.kt | Updated to use TimeFormatter and appSettings.decimalNumbers |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/timer/presentation/components/StatsComponent.kt | Updated to pass decimalNumbers to time formatting |
| composeApp/src/commonMain/kotlin/com/rickyhu/hushtimer/session/presentation/components/SolveCard.kt | Updated to use toFormattedString with appSettings.decimalNumbers |
| composeApp/src/commonMain/composeResources/values/strings.xml | Added "Decimal Numbers" string resource |
| composeApp/src/commonMain/composeResources/values-zh/strings.xml | Added Chinese translation for "Decimal Numbers" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| settings.getIntFlow( | ||
| SettingOptionKeys.DECIMAL_NUMBERS, | ||
| 2, | ||
| ) |
There was a problem hiding this comment.
Similarly, the decimalNumbers flow should validate that persisted values are within valid bounds (2 or 3). If an invalid value is stored, it should be coerced to the default of 2, similar to how penaltyControlType uses a fallback pattern with fromValue(it) ?: PenaltyControlType.BUTTON.
| settings.getIntFlow( | |
| SettingOptionKeys.DECIMAL_NUMBERS, | |
| 2, | |
| ) | |
| settings | |
| .getIntFlow( | |
| SettingOptionKeys.DECIMAL_NUMBERS, | |
| 2, | |
| ).map { value -> | |
| if (value == 2 || value == 3) value else 2 | |
| } |
| is SettingsAction.OnSelectDecimalNumbers -> settingsRepository.setDecimalNumbers(action.decimalNumbers) | ||
| } |
There was a problem hiding this comment.
The formatting issue on line 62 causes inconsistent indentation. The closing braces should be on separate lines for proper code style consistency with the rest of the file.
composeApp/src/commonTest/kotlin/com/rickyhu/hushtimer/utils/TimeFormatterTest.kt
Show resolved
Hide resolved
| var multiplier = 1.0 | ||
| repeat(decimals) { multiplier *= 10 } | ||
| val shifted = value * multiplier | ||
| // Add a tiny epsilon in the direction of the value to correct for floating-point | ||
| // drift where e.g. 1.45 * 10 becomes 14.4999...98 instead of 14.5 | ||
| val epsilon = 1e-10 * if (value < 0) -1 else 1 | ||
| return kotlin.math.round(shifted + epsilon) / multiplier |
There was a problem hiding this comment.
The round function uses a custom epsilon-based rounding implementation that may not handle all edge cases consistently with standard rounding. Consider documenting or adding tests for edge cases like very small epsilon boundaries (e.g., values very close to the midpoint like 1.4500000000001) to ensure the epsilon approach works correctly across different platforms and doesn't introduce unexpected behavior.
| var multiplier = 1.0 | |
| repeat(decimals) { multiplier *= 10 } | |
| val shifted = value * multiplier | |
| // Add a tiny epsilon in the direction of the value to correct for floating-point | |
| // drift where e.g. 1.45 * 10 becomes 14.4999...98 instead of 14.5 | |
| val epsilon = 1e-10 * if (value < 0) -1 else 1 | |
| return kotlin.math.round(shifted + epsilon) / multiplier | |
| val multiplier = 10.0.pow(decimals) | |
| return kotlin.math.round(value * multiplier) / multiplier |
| override fun setDecimalNumbers(value: Int) { | ||
| settings.putInt(SettingOptionKeys.DECIMAL_NUMBERS, value) | ||
| } |
There was a problem hiding this comment.
There's no validation to ensure that decimalNumbers is within valid bounds (2 or 3). If a user manually modifies the persisted settings to an invalid value (e.g., -1, 0, 1, 4, 100), the TimeFormatter methods will still accept and process those values, potentially causing incorrect display formatting. Consider adding validation in setDecimalNumbers to clamp or reject invalid values, or validate the flow in SettingsRepositoryImpl.decimalNumbers to ensure only valid values (2 or 3) are emitted.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
Speedcubers need configurable decimal precision for solve times. This adds a setting to display either 2 or 3 decimal places across all time displays (timer, stats, session history).
TimeResult.finalTimedecimal_numberssetting (options: 2 or 3, default: 2) stored as a plainIntand wired through the full settings stack; fixTimeResult.finalTimeto preserve millisecond precision by rounding to 3 places instead of 2Context / Links
Implementation details
decimal_numbersis stored and exposed as a plainInt(no wrapper enum) — simpler to read, write, and extendSettingsScreenis implemented inline usingListItem+DropdownMenu, showing"2"and"3"as plain text with no string resource translations neededTimeResult.toString()delegates to the newtoFormattedString(numOfDec: Int)method, eliminating duplication and keeping a backward-compatible default of 2PlayerTimerComponent,StatsComponent,SolveCard) readappSettings.decimalNumbersdirectly — no hardcoded2remains in display paths0.0.toFormattedString(appSettings.decimalNumbers)TimeResult.finalTimenow usesround(3)instead ofround(2)to preserve the millisecond precision already captured byTimerViewModel(inWholeMilliseconds / 1000.0); display rounding is handled bytoFormattedStringScreenshots / API changes (if applicable)
2and3as options. Timer, stats, and session cards reflect the selected precision immediately, including a correct non-zero 3rd decimal when 3 decimals is selected.Testing
Steps / commands:
Manual: toggle between 2 and 3 decimals in Settings and verify timer display, stats panel, and session solve cards all update to the correct precision, with the 3rd decimal showing actual millisecond values (not always 0).
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.