Fix timer ignoring workout repetitions - #46
Conversation
|
Nice! :) Could you rebase with main? See how there are two commits? That is what was throwing me for a loop last night. I think the problem here is you already merged the "Following interval" commit, which wasn't authored with Claude, but then you did this without it being rebased with the new merged commit. So this branch you are on doesn't know that main already has this commit. Just a quick rebase of this branch with main should fix it. 🔥 |
The `repetitions` field on `Workout` was never read by the timer. After the last interval finished its rest phase the timer stopped instead of restarting the interval sequence for the remaining repetitions. Also advances to the next workout once all repetitions for the current one are complete. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
da153b2 to
52eb690
Compare
| } | ||
| !isLastWorkout -> { | ||
| currentWorkoutRepetitionIndex = 0 | ||
| currentWorkoutIndex++ |
There was a problem hiding this comment.
This PR actually does something besides fixing repetitions 😮
So you see that right? It noticed that we also were ignoring the fact that we only were iterating over just one Workout, when we could have a whole list of Workout.
That is why this PR has so many lines change, it also includes that.
Could we rename this PR and commit something like Fix timer ignoring multiple workouts and repetitions?
There was a problem hiding this comment.
We were initially focusing on a single workout because each workout already contains multiple intervals and exercises. My intention was that users could select which workout they want to run, which is why I was considering adding a database to manage multiple workouts. What do you think?
There was a problem hiding this comment.
So we will make it so instead of incrementing workouts, you always just "play" one workout at a time, right? Never more than one workout at a time.
Then, because in the future, you will save workouts in a database, it'll be really easy to finish one workout and start another?
I think that makes a lot of sense, and I bet if you fed these instructions into Claude it could kind of put some things together.
One thing is that this would kind of be a big refactor because it probably eliminates the need to have the whole "Build Workout" screen.
That said, I think if you want to merge this as-is it is fine, or you could remove (or tell Claude to remove) the lines about incrementing the workout for now.
| val hasFinishedCurrentWorkoutCycle = isResting && currentIntervalIsLastInWorkout | ||
|
|
||
| if (hasFinishedCurrentWorkoutCycle) { | ||
| val totalRepetitions = currentWorkout?.repetitions ?: 1 | ||
| val hasMoreRepetitions = currentWorkoutRepetitionIndex < totalRepetitions - 1 | ||
| val isLastWorkout = currentWorkoutIndex == workouts.lastIndex |
There was a problem hiding this comment.
Do all these names make sense to you? It got rid of hasFinishedAllWorkouts in favor of isLastWorkout, for example.
| private val currentWorkoutInterval: Interval? | ||
| get() = currentWorkout?.intervals?.getOrNull(index = currentWorkoutIntervalIndex) | ||
|
|
||
| /** Tracks how many times the current workout's intervals have been repeated (0-based). */ |
There was a problem hiding this comment.
| /** Tracks how many times the current workout's intervals have been repeated (0-based). */ | |
| /** The index of the current repetition of the [currentWorkoutInterval] */ |
This is just a nit, but I would change the comment to be like this to be more in line with the others.
Tyler-Lopez
left a comment
Tyler-Lopez
left a comment
There was a problem hiding this comment.
Nice work getting this all working! :) 🔥
Just some small things for you to consider. I would just take a pass through yourself and make sure all of this makes sense. It does look good, it's just that it also included the separate fix to workouts not incrementing too.
Feel free to merge when you want :)
2 participants
Summary
repetitionsdeWorkoutno se leía en el timerTest plan
🤖 Generated with Claude Code