Added session time and stopwatches #24
Conversation
Wrote a function that can break seconds into a formatted string of seconds, minutes, hours, and an overflow of days. This will convert timers into visible text.
Added a member variable that will be updated with UnityEngine::Time::get_realtimeSinceStartup. Could just use the Unity function over and over again, but ClockUpdater seems like a hub for time information.
Display the session time underneath the timezone and UTC time. Showing all time options on the info section of the settings menu should help convey what each of them are.
The ClockType is stored as an int in the mod config but can be casted to an enum that is defined in ClockValues.hpp. Seems like the correct place for it.
Oh cool config-utils just has an enum dropdown function already.
Could be an indent, but it was so small that it just looked like a mistake to me.
When session time is selected, it is now displayed on the floating clock.
I like having units on the time
Renamed custom timer to stopwatch, uncommented code in ClockUpdater and ClockValues from previous outlining, and added it to the information area.
Use Unity's timing system rather than the clock function in ctime. clock is based on CPU time rather than real time, and CLOCKS_PER_SECOND is an inaccurate constexpr. The stopwatch was going a little bit faster than it should have been.
Duration 0 will now show as 0:00 instead of 00:00. The double zero should only appear if another number comes before, such as 1:00:00. Looks and reads nicer.
Make it possible to control the stopwatch with UI.
Renamed stopwatch to stopwatch 1 and duplicated all of the code to make stopwatch 2 as well. Stopwatch could be used to remove crash vulnerability in session time, but could also track all time usage. So hard to decide... Why not both! Or do whatever!
Quickened the save interval from every 10 seconds to every 5 seconds. The stopwatched isn't saved while in a beatmap for performace, and it takes less than 10 seconds to exit the game after finishing a beatmap. This should reduce the chance of time getting lost.
This number appears to have gotten left behind at 1.11.0-Dev. New features should mean new version.
Additions were originally named timers, but midway through I realized that stopwatch was a more accurate name. Fixed a few variable and function names to use the updated term.
WalkthroughAdds session and two stopwatch timers, a ClockType enum and labels, five new config fields for clock selection and stopwatch state, ClockUpdater timers/formatting/getters/resetters with periodic save, UI controls (dropdown, pause/start, reset), and a package version bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as ClockViewController
participant Config as ModConfig
participant Updater as ClockUpdater
participant Time as Time API
rect rgb(245,250,255)
UI->>Config: Read ClockType, Stopwatch*Seconds, *Paused
UI->>Updater: Start()
Updater->>Config: Load Stopwatch*Seconds
end
loop Frame update
Updater->>Time: get_realtimeSinceStartup()
Time-->>Updater: now
alt Stopwatch1 not paused
Updater->>Updater: accumulate stopwatch1Seconds
end
alt Stopwatch2 not paused
Updater->>Updater: accumulate stopwatch2Seconds
end
Updater->>Updater: accumulate sessionTimeSeconds
opt periodic save
Updater->>Config: Save Stopwatch*Seconds
end
UI->>Updater: get*Seconds()
Updater-->>UI: values
UI->>Updater: getStopwatchString(totalSeconds)
Updater-->>UI: formatted string
UI->>UI: render based on ClockType
end
rect rgb(235,255,235)
User->>UI: Toggle Pause/Start (Stopwatch1/2)
UI->>Config: Flip Stopwatch*Paused
User->>UI: Reset Stopwatch1/2
UI->>Updater: resetStopwatch1/2()
UI->>Config: Set Stopwatch*Seconds = 0
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
shared/ClockUpdater.hpp (2)
43-47: Avoid const on value returns; add include.Returning const by value has no effect and can hinder move semantics. Also include for the static formatter declaration.
Apply:
@@ - const double getSessionTimeSeconds() const; - const double getStopwatch1Seconds() const; - const double getStopwatch2Seconds() const; + double getSessionTimeSeconds() const; + double getStopwatch1Seconds() const; + double getStopwatch2Seconds() const;And at the top of the file:
@@ #include <ctime> #include <time.h> +#include <string>
52-52: Consistent signature style.Drop the const on a by-value parameter for consistency with common style.
Apply:
- static std::string getStopwatchString(const double totalSeconds); + static std::string getStopwatchString(double totalSeconds);src/ClockViewContoller.cpp (1)
96-111: Persist on toggle/reset to reduce config write cadence.If ClockUpdater saves periodically, also persisting immediately on Pause/Start and Reset improves resilience across crashes and reduces timer-driven writes.
Optionally call your config save/write API after toggling or after reset.
include/ClockModConfig.hpp (2)
7-8: Avoid truncation: persist stopwatch seconds as floating-point, not int.Stopwatches accumulate as doubles in ClockUpdater.cpp, but the config stores them as ints, losing fractional seconds at every save/load. Persist as float (or double) to keep precision while keeping config size small.
- CONFIG_VALUE(Stopwatch1Seconds, int, "Stopwatch 1 Seconds", 0, "The saved time on stopwatch 1."); - CONFIG_VALUE(Stopwatch2Seconds, int, "Stopwatch 2 Seconds", 0, "The saved time on stopwatch 2."); + CONFIG_VALUE(Stopwatch1Seconds, float, "Stopwatch 1 Seconds", 0.0f, "The saved time on stopwatch 1."); + CONFIG_VALUE(Stopwatch2Seconds, float, "Stopwatch 2 Seconds", 0.0f, "The saved time on stopwatch 2.");
6-6: Consider validating ClockType on read.ClockType is stored as an int. Add a simple clamp (e.g., 0..3) when reading to prevent out-of-range values from older configs or manual edits.
src/ClockUpdater.cpp (5)
57-73: Formatting helper is solid; minor edge-case polish optional.Current output is compact and correct. Optional: clamp totalSeconds to >= 0 to avoid negative displays if the engine time ever jitters, and optionally pad days for very long sessions.
- std::string ClockUpdater::getStopwatchString(const double totalSeconds) { + std::string ClockUpdater::getStopwatchString(const double totalSecondsIn) { + const double totalSeconds = std::max(0.0, totalSecondsIn);
158-159: Type mismatch will drop precision on load if config stays int.If config remains int, these assignments will silently truncate on save/load cycles. Switching the config fields to float (see header comment) resolves this; otherwise, explicitly cast with a comment to acknowledge precision loss.
226-233: Make persistence interval frame-rate independent and self-documenting.Using a counter tied to the 0.25s gate is fine but brittle. Prefer a realtime check with a named interval; also guarantees a final save if updates slow down.
- // Scuffed, but not any more than the rest of this codebase - static int stopwatchSaveTimer = 0; - stopwatchSaveTimer++; - if(stopwatchSaveTimer > 5 / NUM_SECONDS && !Config.IsInSong) { // Avoid dropping frames during gameplay - getModConfig().Stopwatch1Seconds.SetValue(stopwatch1Seconds); - getModConfig().Stopwatch2Seconds.SetValue(stopwatch2Seconds); - stopwatchSaveTimer = 0; - } + // Persist every N seconds based on realtime (avoids frame/tick coupling) + constexpr double kPersistIntervalSec = 5.0; + static double lastPersist = 0.0; + if (lastPersist == 0.0) lastPersist = sessionTimeSeconds; + if (!Config.IsInSong && (sessionTimeSeconds - lastPersist) >= kPersistIntervalSec) { + getModConfig().Stopwatch1Seconds.SetValue(stopwatch1Seconds); + getModConfig().Stopwatch2Seconds.SetValue(stopwatch2Seconds); + lastPersist = sessionTimeSeconds; + }
248-253: ClockType routing works; small readability tweak optional.A switch on ClockType (with a default to current time) improves readability and makes it harder to miss a future enum value.
- if(!_message.empty()) clockresult = _message; - else if(getModConfig().ClockType.GetValue() == static_cast<int>(ClockTypes::SessionTime)) clockresult = getStopwatchString(sessionTimeSeconds); - else if(getModConfig().ClockType.GetValue() == static_cast<int>(ClockTypes::Stopwatch1)) clockresult = getStopwatchString(stopwatch1Seconds); - else if(getModConfig().ClockType.GetValue() == static_cast<int>(ClockTypes::Stopwatch2)) clockresult = getStopwatchString(stopwatch2Seconds); - else clockresult = getTimeString((struct tm*)timeinfo); + if(!_message.empty()) { + clockresult = _message; + } else { + switch (static_cast<ClockTypes>(getModConfig().ClockType.GetValue())) { + case ClockTypes::SessionTime: clockresult = getStopwatchString(sessionTimeSeconds); break; + case ClockTypes::Stopwatch1: clockresult = getStopwatchString(stopwatch1Seconds); break; + case ClockTypes::Stopwatch2: clockresult = getStopwatchString(stopwatch2Seconds); break; + default: clockresult = getTimeString((struct tm*)timeinfo); break; + } + }
300-318: Getters/resetters look correct. Consider flushing on teardown to avoid lost progress.Given periodic saves skip during songs, add a final flush in OnDisable/OnDestroy so recent stopwatch progress isn’t lost if the app quits mid-song or shortly after.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
include/ClockModConfig.hpp(1 hunks)include/ClockValues.hpp(1 hunks)qpm.json(1 hunks)qpm.shared.json(1 hunks)shared/ClockUpdater.hpp(2 hunks)src/ClockUpdater.cpp(6 hunks)src/ClockViewContoller.cpp(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/ClockViewContoller.cpp (1)
src/ClockUpdater.cpp (6)
getTimeFormat(25-47)getTimeFormat(25-25)getInstance(330-332)getInstance(330-330)getStopwatchString(58-73)getStopwatchString(58-58)
src/ClockUpdater.cpp (1)
shared/ClockUpdater.hpp (6)
getStopwatchString(52-52)getSessionTimeSeconds(43-43)getStopwatch1Seconds(44-44)getStopwatch2Seconds(45-45)resetStopwatch1(46-46)resetStopwatch2(47-47)
🔇 Additional comments (7)
qpm.json (1)
9-9: Version bump verified. qpm.json and qpm.shared.json info.version both set to 1.15.0-Dev; no other version literals require updating.qpm.shared.json (1)
10-10: Synced shared config version.qpm.json and qpm.shared.json align at 1.15.0-Dev. No further action.
shared/ClockUpdater.hpp (1)
30-33: Nit: initialize-and-own – fine.The new counters are straightforward and self-explanatory.
src/ClockViewContoller.cpp (1)
127-127: Dropdown labels source is good; ensure lifetime and ordering.clockTypeStrs is a static with static storage duration; passing it is safe. With the constexpr array/static_assert change proposed, ordering will stay aligned with ClockTypes.
include/ClockModConfig.hpp (1)
9-10: Defaults make sense.Starting both stopwatches paused by default is sane and avoids silent background accumulation.
src/ClockUpdater.cpp (2)
13-13: Correct include.Including UnityEngine/Time.hpp is required for realtimeSinceStartup; good catch.
209-219: Delta accumulation placement is good.Incrementing stopwatches before the 0.25s gate preserves accuracy irrespective of UI update cadence.
Classes that are included from another include shouldn't be relied upon. Include string_view manually in case it disappears from the two UnityEngine includes in the future. Jokes on you CodeRabbit, I wrote each of those 22 characters on my own keyboard >:P
|
Looks all good to me |
2 participants
Someone requested an elapsed time mod in the BSMG Discord server, something to replace Move after it was discontinued. I liked the idea and figured it would fit in nicely into Clock Mod.
ClockViewControllerThe stopwatches are useful for recording session time without crashes, emulating the discontinued Move app, timing how long it takes to achieve some goal, etc.
I was going to follow whatever style guide was used in the rest of the mod, but consistency appears to have already been thrown out the window...
Oh I also updated the mod version from v1.11.0-Dev to v1.15.0-Dev in the qpm files. Seems like that step was forgotten in the previous bump versions.
Summary by CodeRabbit
New Features
UI/UX
Chores