From c0d6ad868dcf90dccfc477faae74ecf6109c53c1 Mon Sep 17 00:00:00 2001 From: Dharanish Date: Mon, 24 Jun 2024 15:14:43 +0200 Subject: [PATCH] Fix skip days scores and targets --- .../habits/edit/EditHabitActivity.kt | 11 ++--- .../isoron/uhabits/core/models/EntryList.kt | 46 +++-------------- .../org/isoron/uhabits/core/models/Habit.kt | 6 +-- .../isoron/uhabits/core/models/SkipDays.kt | 4 ++ .../isoron/uhabits/core/models/StreakList.kt | 17 +++++-- .../isoron/uhabits/core/models/WeekdayList.kt | 8 ++- .../core/models/sqlite/SQLiteEntryList.kt | 2 +- .../screens/habits/show/views/SubtitleCard.kt | 2 +- .../screens/habits/show/views/TargetCard.kt | 49 ++++++++++--------- 9 files changed, 65 insertions(+), 80 deletions(-) diff --git a/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/edit/EditHabitActivity.kt b/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/edit/EditHabitActivity.kt index af8c85523..e90bd0295 100644 --- a/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/edit/EditHabitActivity.kt +++ b/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/edit/EditHabitActivity.kt @@ -303,7 +303,6 @@ class EditHabitActivity : AppCompatActivity() { } habit.frequency = Frequency(freqNum, freqDen) - if (freqDen != 1) isSkipDays = false habit.skipDays = SkipDays(isSkipDays, listSkipDays) if (habitType == HabitType.NUMERICAL) { habit.targetValue = binding.targetInput.text.toString().toDouble() @@ -360,9 +359,11 @@ class EditHabitActivity : AppCompatActivity() { private fun populateSkipDays() { val preferences = (application as HabitsApplication).component.preferences - if (preferences.isSkipEnabled || isSkipDays) { + if (preferences.isSkipEnabled && (freqDen == 1 || freqDen == 7)) { binding.skipDaysOuterBox.visibility = View.VISIBLE } else { + isSkipDays = false + listSkipDays = WeekdayList.NO_DAY binding.skipDaysOuterBox.visibility = View.GONE } if (isSkipDays) { @@ -381,11 +382,7 @@ class EditHabitActivity : AppCompatActivity() { 30 -> getString(R.string.every_month) else -> "$freqNum/$freqDen" } - if (freqDen == 1) { - binding.skipDaysOuterBox.visibility = View.VISIBLE - } else { - binding.skipDaysOuterBox.visibility = View.GONE - } + populateSkipDays() } private fun populateTargetType() { diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/EntryList.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/EntryList.kt index 12ba3dd2a..f44a88658 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/EntryList.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/EntryList.kt @@ -24,7 +24,6 @@ import org.isoron.uhabits.core.models.Entry.Companion.UNKNOWN import org.isoron.uhabits.core.models.Entry.Companion.YES_AUTO import org.isoron.uhabits.core.models.Entry.Companion.YES_MANUAL import org.isoron.uhabits.core.utils.DateUtils -import java.util.ArrayList import java.util.Calendar import javax.annotation.concurrent.ThreadSafe import kotlin.collections.set @@ -91,18 +90,16 @@ open class EntryList { open fun recomputeFrom( originalEntries: EntryList, frequency: Frequency, - isNumerical: Boolean, - skipDays: SkipDays + isNumerical: Boolean ) { clear() val original = originalEntries.getKnown() if (isNumerical) { - val computed = addEntriesWithSkipDays(original, skipDays) - computed.forEach { add(it) } + original.forEach { add(it) } } else { val intervals = buildIntervals(frequency, original) snapIntervalsTogether(intervals) - val computed = buildEntriesFromInterval(original, intervals, skipDays) + val computed = buildEntriesFromInterval(original, intervals) computed.filter { it.value != UNKNOWN || it.notes.isNotEmpty() }.forEach { add(it) } } } @@ -170,8 +167,7 @@ open class EntryList { */ fun buildEntriesFromInterval( original: List, - intervals: List, - skipDays: SkipDays + intervals: List ): List { val result = arrayListOf() if (original.isEmpty()) return result @@ -200,7 +196,7 @@ open class EntryList { current = interval.end while (current >= interval.begin) { val offset = current.daysUntil(to) - result[offset] = if (skipDays.isDaySkipped(current)) Entry(current, SKIP) else Entry(current, YES_AUTO) + result[offset] = Entry(current, YES_AUTO) current = current.minus(1) } } @@ -208,9 +204,7 @@ open class EntryList { // Copy original entries except for the skipped days original.forEach { entry -> val offset = entry.timestamp.daysUntil(to) - val value = if (skipDays.isDaySkipped(entry)) { - SKIP - } else if ( + val value = if ( result[offset].value == UNKNOWN || entry.value == SKIP || entry.value == YES_MANUAL @@ -277,29 +271,6 @@ open class EntryList { } return intervals } - - fun addEntriesWithSkipDays(original: List, skipDays: SkipDays): List { - if (original.isEmpty()) return original - val earliest = original.last().timestamp - val today = DateUtils.getTodayWithOffset() - val computed = mutableListOf() - var current = today - var offset = 0 - while (current >= earliest) { - if (current == original[offset].timestamp) { - if (!skipDays.isDaySkipped(current)) { - computed.add(original[offset]) - } else { - computed.add(Entry(current, SKIP)) - } - offset++ - } else if (skipDays.isDaySkipped(current)) { - computed.add(Entry(current, SKIP)) - } - current = current.minus(1) - } - return computed - } } } @@ -352,12 +323,11 @@ fun List.groupedSum( */ fun List.countSkippedDays( truncateField: DateUtils.TruncateField, - firstWeekday: Int = Calendar.SATURDAY, - skipDays: SkipDays + firstWeekday: Int = Calendar.SATURDAY ): List { val thisIntervalStart = DateUtils.getTodayWithOffset().truncate(truncateField, firstWeekday) return this.map { (timestamp, value) -> - if (value == SKIP || skipDays.isDaySkipped(timestamp)) { + if (value == SKIP) { Entry(timestamp, 1) } else { Entry(timestamp, 0) diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/Habit.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/Habit.kt index 132250754..8c87a038a 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/Habit.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/Habit.kt @@ -79,8 +79,7 @@ data class Habit( computedEntries.recomputeFrom( originalEntries = originalEntries, frequency = frequency, - isNumerical = isNumerical, - skipDays = skipDays + isNumerical = isNumerical ) val today = DateUtils.getTodayWithOffset() @@ -103,7 +102,8 @@ data class Habit( streaks.recompute( computedEntries, from, - to + to, + skipDays ) } diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/SkipDays.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/SkipDays.kt index 4d386fe59..94ad521f6 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/SkipDays.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/SkipDays.kt @@ -34,6 +34,10 @@ data class SkipDays( return isSkipDays && days.isDayTrue(entry.timestamp.weekday) } + fun numDaysSkipped(): Int { + return if (isSkipDays) days.numDays() else 0 + } + companion object { @JvmField val NONE = SkipDays(false, WeekdayList(0)) diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/StreakList.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/StreakList.kt index d3bed0af7..e6709d416 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/StreakList.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/StreakList.kt @@ -37,16 +37,23 @@ class StreakList { fun recompute( computedEntries: EntryList, from: Timestamp, - to: Timestamp + to: Timestamp, + skipDays: SkipDays ) { list.clear() val timestamps = computedEntries - .getByInterval(from, to) + .getByInterval(from, to, skipDays) .filter { it.value > 0 } .map { it.timestamp } .toTypedArray() + val values = computedEntries + .getByInterval(from, to, skipDays) + .filter { it.value > 0 } + .map { it.value } + .toTypedArray() if (timestamps.isEmpty()) return + var notSkipStreak = false var begin = timestamps[0] var end = timestamps[0] @@ -54,12 +61,14 @@ class StreakList { val current = timestamps[i] if (current == begin.minus(1)) { begin = current + if (values[i] != Entry.SKIP) notSkipStreak = true } else { - list.add(Streak(begin, end)) + if (notSkipStreak) list.add(Streak(begin, end)) begin = current end = current + notSkipStreak = (values[i] != Entry.SKIP) } } - list.add(Streak(begin, end)) + if (notSkipStreak) list.add(Streak(begin, end)) } } diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/WeekdayList.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/WeekdayList.kt index 1430a7dc9..212e8cee2 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/WeekdayList.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/WeekdayList.kt @@ -38,10 +38,10 @@ class WeekdayList { this.weekdays = Arrays.copyOf(weekdays, 7) } - constructor(addDays: BooleanArray, removeDays: BooleanArray) { + constructor(days: BooleanArray, removeDays: BooleanArray) { weekdays = BooleanArray(7) for (i in 0..6) { - weekdays[i] = addDays[i] && !removeDays[i] + weekdays[i] = days[i] && !removeDays[i] } } @@ -69,6 +69,10 @@ class WeekdayList { return weekdays[dayNum] } + fun numDays(): Int { + return weekdays.count { it } + } + override fun equals(other: Any?): Boolean { if (this === other) return true if (other == null || javaClass != other.javaClass) return false diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/sqlite/SQLiteEntryList.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/sqlite/SQLiteEntryList.kt index 824234ad2..013a071ca 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/sqlite/SQLiteEntryList.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/models/sqlite/SQLiteEntryList.kt @@ -78,7 +78,7 @@ class SQLiteEntryList(database: Database) : EntryList() { return super.getKnown() } - override fun recomputeFrom(originalEntries: EntryList, frequency: Frequency, isNumerical: Boolean, skipDays: SkipDays) { + override fun recomputeFrom(originalEntries: EntryList, frequency: Frequency, isNumerical: Boolean) { throw UnsupportedOperationException() } diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/SubtitleCard.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/SubtitleCard.kt index 01784e2f2..3c3c9b43c 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/SubtitleCard.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/SubtitleCard.kt @@ -31,7 +31,7 @@ data class SubtitleCardState( val color: PaletteColor, val frequency: Frequency, val isNumerical: Boolean, - val skipDays: SkipDays, + val skipDays: SkipDays = SkipDays.NONE, val question: String, val reminder: Reminder?, val targetValue: Double = 0.0, diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/TargetCard.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/TargetCard.kt index 43a240dc6..a1e00a30f 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/TargetCard.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/show/views/TargetCard.kt @@ -26,7 +26,6 @@ import org.isoron.uhabits.core.models.countSkippedDays import org.isoron.uhabits.core.models.groupedSum import org.isoron.uhabits.core.ui.views.Theme import org.isoron.uhabits.core.utils.DateUtils -import java.util.ArrayList import java.util.Calendar import kotlin.math.max @@ -48,59 +47,55 @@ class TargetCardPresenter { val today = DateUtils.getTodayWithOffset() val (yearBegin, yearEnd) = getYearRange(firstWeekday) val oldest = habit.computedEntries.getKnown().lastOrNull()?.timestamp ?: today - val entriesForSkip = habit.computedEntries.getByInterval(yearBegin, yearEnd, habit.skipDays) - val entriesForSum = habit.computedEntries.getByInterval(oldest, today) + val entriesWithSkip = habit.computedEntries.getByInterval(yearBegin, yearEnd, habit.skipDays) + val entries = habit.computedEntries.getByInterval(oldest, today) + val entriesForCount = if (habit.frequency.denominator == 1) entriesWithSkip else entries - val valueToday = entriesForSum.groupedSum( + val valueToday = entries.groupedSum( truncateField = DateUtils.TruncateField.DAY, isNumerical = habit.isNumerical ).firstOrNull()?.value ?: 0 - val skippedDayToday = entriesForSkip.countSkippedDays( - truncateField = DateUtils.TruncateField.DAY, - skipDays = habit.skipDays + val skippedDayToday = entriesWithSkip.countSkippedDays( + truncateField = DateUtils.TruncateField.DAY ).firstOrNull()?.value ?: 0 - val valueThisWeek = entriesForSum.groupedSum( + val valueThisWeek = entries.groupedSum( truncateField = DateUtils.TruncateField.WEEK_NUMBER, firstWeekday = firstWeekday, isNumerical = habit.isNumerical ).firstOrNull()?.value ?: 0 - val skippedDaysThisWeek = entriesForSkip.countSkippedDays( + val skippedDaysThisWeek = entriesForCount.countSkippedDays( truncateField = DateUtils.TruncateField.WEEK_NUMBER, - firstWeekday = firstWeekday, - skipDays = habit.skipDays + firstWeekday = firstWeekday ).firstOrNull()?.value ?: 0 - val valueThisMonth = entriesForSum.groupedSum( + val valueThisMonth = entries.groupedSum( truncateField = DateUtils.TruncateField.MONTH, isNumerical = habit.isNumerical ).firstOrNull()?.value ?: 0 - val skippedDaysThisMonth = entriesForSkip.countSkippedDays( - truncateField = DateUtils.TruncateField.MONTH, - skipDays = habit.skipDays + val skippedDaysThisMonth = entriesForCount.countSkippedDays( + truncateField = DateUtils.TruncateField.MONTH ).firstOrNull()?.value ?: 0 - val valueThisQuarter = entriesForSum.groupedSum( + val valueThisQuarter = entries.groupedSum( truncateField = DateUtils.TruncateField.QUARTER, isNumerical = habit.isNumerical ).firstOrNull()?.value ?: 0 - val skippedDaysThisQuarter = entriesForSkip.countSkippedDays( - truncateField = DateUtils.TruncateField.QUARTER, - skipDays = habit.skipDays + val skippedDaysThisQuarter = entriesForCount.countSkippedDays( + truncateField = DateUtils.TruncateField.QUARTER ).firstOrNull()?.value ?: 0 - val valueThisYear = entriesForSum.groupedSum( + val valueThisYear = entries.groupedSum( truncateField = DateUtils.TruncateField.YEAR, isNumerical = habit.isNumerical ).firstOrNull()?.value ?: 0 - val skippedDaysThisYear = entriesForSkip.countSkippedDays( - truncateField = DateUtils.TruncateField.YEAR, - skipDays = habit.skipDays + val skippedDaysThisYear = entriesForCount.countSkippedDays( + truncateField = DateUtils.TruncateField.YEAR ).firstOrNull()?.value ?: 0 val cal = DateUtils.getStartOfTodayCalendarWithOffset() @@ -114,8 +109,14 @@ class TargetCardPresenter { val monthsInQuarter = 3 val monthsInYear = 12 + val effectiveDenominator = if (habit.frequency.denominator == 7 && habit.skipDays.isSkipDays) { + habit.frequency.denominator - habit.skipDays.numDaysSkipped() + } else { + habit.frequency.denominator + } + val denominator = habit.frequency.denominator - val dailyTarget = habit.targetValue / habit.frequency.denominator + val dailyTarget = habit.targetValue / effectiveDenominator var targetToday = dailyTarget var targetThisWeek = when (denominator) {