From ab28216280e7f8f71dae4d7022afc6fe20100275 Mon Sep 17 00:00:00 2001 From: "Philipp Wagner (DN9APW)" Date: Sun, 11 Oct 2026 01:03:59 +0200 Subject: [PATCH] fix: sort the AP column numerically instead of by its text (#87) The "AP [minutes / pot%]" column of the station list and of the directed messages had no comparator, so DataTableState fell back to the rendered text and "13 (75%)" sorted above "3 (50%)". AirplaneCell now owns a shared comparator, AirplaneCell.order, that both column definitions use. The sort key is the first aeroplane of the cell, which is the one the operator reads and the one the colour marks: ascending by its arriving minutes, then by the better potential first for equal minutes. A null info (nothing computed) and an empty list ("nil") carry Int.MAX_VALUE, so they sort after every real aeroplane in ascending order, consistent with the qrb and qtf sentinels. The second aeroplane of a cell does not influence the order. Cell text, colours, column ids, titles and weights are unchanged, as are DataTableState and the sort cycle. Fixes #87 Co-Authored-By: Claude Fable 5.1 --- .../kst4contest/view/compose/AirplaneCell.kt | 22 +++++++ .../view/compose/DirectedMessageColumns.kt | 1 + .../view/compose/StationColumns.kt | 1 + .../view/compose/AirplaneCellTest.kt | 65 +++++++++++++++++++ .../compose/DirectedMessageColumnsTest.kt | 28 ++++++++ .../view/compose/StationColumnsTest.kt | 29 +++++++++ github_docs/de-Changelog.md | 2 + github_docs/en-Changelog.md | 2 + 8 files changed, 150 insertions(+) diff --git a/app-desktop/src/main/kotlin/kst4contest/view/compose/AirplaneCell.kt b/app-desktop/src/main/kotlin/kst4contest/view/compose/AirplaneCell.kt index b27e981d..188eae16 100644 --- a/app-desktop/src/main/kotlin/kst4contest/view/compose/AirplaneCell.kt +++ b/app-desktop/src/main/kotlin/kst4contest/view/compose/AirplaneCell.kt @@ -1,6 +1,7 @@ package kst4contest.view.compose import androidx.compose.ui.graphics.Color +import kst4contest.model.AirPlane import kst4contest.model.AirPlaneReflectionInfo /** @@ -34,6 +35,27 @@ object AirplaneCell { */ private const val NOTHING_COMING = "nil" + // ---- the order ---------------------------------------------------------- + + /** Unknown or no aeroplane sorts after every real one. */ + private const val NO_PLANE = Int.MAX_VALUE + + /** The aeroplane the cell shows first; the parser already put the best one there. */ + private fun first(info: AirPlaneReflectionInfo?): AirPlane? = info?.risingAirplanes?.firstOrNull() + + /** + * Ascending: the minutes the operator reads first, then the better potential first. + * + * Issue #87: without a comparator the table fell back to the rendered text, which put + * "13 (75%)" above "3 (50%)". Only the first aeroplane counts, as it is the one the + * operator reads and the one the colour marks; the second is in the cell for context. + * Empty and "nil" cells carry [NO_PLANE]: like qrb and qtf, an absent value gets a + * sentinel instead of a null check, here the maximum so the cell sorts last ascending. + */ + val order: Comparator = + compareBy { first(it)?.arrivingDurationMinutes ?: NO_PLANE } + .thenByDescending { first(it)?.potential ?: 0 } + /** `.table-cell-100PercentAP`, identical in both stylesheets: a certain path. */ val PURPLE_100 = Color(0xFFF98AFF) diff --git a/app-desktop/src/main/kotlin/kst4contest/view/compose/DirectedMessageColumns.kt b/app-desktop/src/main/kotlin/kst4contest/view/compose/DirectedMessageColumns.kt index 19f2b714..33e9cb8f 100644 --- a/app-desktop/src/main/kotlin/kst4contest/view/compose/DirectedMessageColumns.kt +++ b/app-desktop/src/main/kotlin/kst4contest/view/compose/DirectedMessageColumns.kt @@ -57,6 +57,7 @@ object DirectedMessageColumns { title = "AP [minutes / pot%]", value = { AirplaneCell.format(it.sender?.airPlaneReflectInfo) }, accent = { _, rendered -> AirplaneCell.accentFor(rendered) }, + comparator = compareBy(AirplaneCell.order) { it.sender?.airPlaneReflectInfo }, weight = 1.4f, ), DataColumn( diff --git a/app-desktop/src/main/kotlin/kst4contest/view/compose/StationColumns.kt b/app-desktop/src/main/kotlin/kst4contest/view/compose/StationColumns.kt index 13f5ca8e..b9065c35 100644 --- a/app-desktop/src/main/kotlin/kst4contest/view/compose/StationColumns.kt +++ b/app-desktop/src/main/kotlin/kst4contest/view/compose/StationColumns.kt @@ -55,6 +55,7 @@ object StationColumns { title = "AP [minutes / pot%]", value = { AirplaneCell.format(it.airPlaneReflectInfo) }, accent = { _, rendered -> AirplaneCell.accentFor(rendered) }, + comparator = compareBy(AirplaneCell.order) { it.airPlaneReflectInfo }, weight = 1.4f, )) add(DataColumn(id = "tropo", title = "Tropo", value = tropoOf, weight = 0.7f)) diff --git a/app-desktop/src/test/kotlin/kst4contest/view/compose/AirplaneCellTest.kt b/app-desktop/src/test/kotlin/kst4contest/view/compose/AirplaneCellTest.kt index d5a1067c..77d30c01 100644 --- a/app-desktop/src/test/kotlin/kst4contest/view/compose/AirplaneCellTest.kt +++ b/app-desktop/src/test/kotlin/kst4contest/view/compose/AirplaneCellTest.kt @@ -4,6 +4,7 @@ import kst4contest.model.AirPlane import kst4contest.model.AirPlaneReflectionInfo import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame import org.junit.jupiter.api.Test /** @@ -97,6 +98,70 @@ class AirplaneCellTest { assertEquals(AirplaneCell.RED_75, AirplaneCell.accentFor("7 (50%) / 12 (75%)")?.foreground) } + // ---- the order ---------------------------------------------------------- + + /** + * Issue #87: the column was sorted by its rendered text, so "13 (75%)" came before + * "3 (50%)". These are the thirteen cells of the screenshot in the issue, in the order + * they were shown; sorted, the minutes have to read as numbers. + */ + @Test + fun `the minutes sort as numbers, not as text`() { + val sorted = screenshotCells().sortedWith(AirplaneCell.order) + + assertEquals( + listOf(1, 1, 3, 3, 6, 6, 8, 8, 9, 9, 13, 14, 19), + sorted.map { it.risingAirplanes.first().arrivingDurationMinutes }, + ) + } + + /** Same minutes: the better potential first, which is also the one the colour marks. */ + @Test + fun `equal minutes put the better potential first`() { + val sorted = screenshotCells().sortedWith(AirplaneCell.order) + + assertEquals( + listOf("3 (75%)", "3 (50%)"), + sorted.map { AirplaneCell.format(it) }.filter { it.startsWith("3 ") }, + "the screenshot had 3 (50%) above 3 (75%); the order is not just stable", + ) + } + + /** + * An empty cell (nothing computed) and "nil" (no aeroplane coming) both carry the + * maximum sentinel, so ascending puts them after the last real aeroplane. + */ + @Test + fun `no aeroplane sorts after the last real one`() { + val nothingComputed: AirPlaneReflectionInfo? = null + val nothingComing = reflectionInfo() + val cells = listOf(nothingComputed, nothingComing) + screenshotCells() + + val sorted = cells.sortedWith(AirplaneCell.order) + + assertEquals(19, sorted[sorted.size - 3]?.risingAirplanes?.first()?.arrivingDurationMinutes) + assertSame(nothingComputed, sorted[sorted.size - 2]) + assertSame(nothingComing, sorted[sorted.size - 1]) + } + + /** Only the first aeroplane, the one the operator reads first, decides. */ + @Test + fun `the second aeroplane of a cell does not influence the order`() { + val nineThenOne = reflectionInfo(plane(9, 75), plane(1, 100)) + val threeThenTwenty = reflectionInfo(plane(3, 50), plane(20, 50)) + + val sorted = listOf(nineThenOne, threeThenTwenty).sortedWith(AirplaneCell.order) + + assertSame(threeThenTwenty, sorted[0]) + assertSame(nineThenOne, sorted[1]) + } + + /** The AP column of the issue's screenshot, top to bottom: (minutes, potential). */ + private fun screenshotCells(): List = listOf( + 1 to 75, 1 to 75, 13 to 75, 14 to 50, 19 to 50, 3 to 50, 3 to 75, + 6 to 75, 6 to 75, 8 to 75, 8 to 75, 9 to 75, 9 to 75, + ).map { (minutes, potential) -> reflectionInfo(plane(minutes, potential)) } + private fun reflectionInfo(vararg planes: AirPlane): AirPlaneReflectionInfo = AirPlaneReflectionInfo().apply { risingAirplanes = planes.toMutableList() } diff --git a/app-desktop/src/test/kotlin/kst4contest/view/compose/DirectedMessageColumnsTest.kt b/app-desktop/src/test/kotlin/kst4contest/view/compose/DirectedMessageColumnsTest.kt index 8729bfa8..a5f8f926 100644 --- a/app-desktop/src/test/kotlin/kst4contest/view/compose/DirectedMessageColumnsTest.kt +++ b/app-desktop/src/test/kotlin/kst4contest/view/compose/DirectedMessageColumnsTest.kt @@ -6,6 +6,7 @@ import kst4contest.model.ChatCategory import kst4contest.model.ChatMember import kst4contest.model.ChatMessage import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test @@ -98,6 +99,33 @@ class DirectedMessageColumnsTest { assertEquals("7 (85%)", cell("airscout", message(sender = sender))) } + /** + * Issue #87: the AP column had no comparator and fell back to its text, so "13 (75%)" + * sorted before "9 (75%)". Same shape as DataTableStateTest's "a column with a + * comparator sorts by it, not by its text", with the real columns. + */ + @Test + fun `airscout sorts numerically`() { + assertNotNull(columns.single { it.id == "airscout" }.comparator, "the AP column sorts by its own key") + + val table = DataTableState(columns = columns, rowKey = RowKeys.byReference()) + table.replaceRows(listOf( + message(sender = memberWithAeroplane("DL13ABC", minutes = 13)), + message(sender = memberWithAeroplane("DL9ABC", minutes = 9)), + )) + + table.toggleSort("airscout") + + assertEquals(listOf("DL9ABC", "DL13ABC"), table.visibleRows.map { it.sender?.callSign }, + "9 before 13 numerically; by text it would be the other way round") + } + + private fun memberWithAeroplane(call: String, minutes: Int): ChatMember = member(call).apply { + airPlaneReflectInfo = AirPlaneReflectionInfo().apply { + risingAirplanes = mutableListOf(AirPlane().apply { arrivingDurationMinutes = minutes; potential = 75 }) + } + } + @Test fun `the category names the chat the message arrived in`() { val message = message(sender = member("DL0ABC")).apply { chatCategory = ChatCategory(2) } diff --git a/app-desktop/src/test/kotlin/kst4contest/view/compose/StationColumnsTest.kt b/app-desktop/src/test/kotlin/kst4contest/view/compose/StationColumnsTest.kt index 5afcc76c..4ee027cb 100644 --- a/app-desktop/src/test/kotlin/kst4contest/view/compose/StationColumnsTest.kt +++ b/app-desktop/src/test/kotlin/kst4contest/view/compose/StationColumnsTest.kt @@ -1,9 +1,12 @@ package kst4contest.view.compose +import kst4contest.model.AirPlane +import kst4contest.model.AirPlaneReflectionInfo import kst4contest.model.ChatMember import org.junit.jupiter.api.Test import org.junit.jupiter.api.Assertions.assertDoesNotThrow import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue @@ -120,6 +123,32 @@ class StationColumnsTest { assertEquals("", columns().first { it.id == "not-qrv" }.value(ChatMember())) } + /** + * Issue #87: the AP column had no comparator and fell back to its text, so "13 (75%)" + * sorted before "9 (75%)". Same shape as DataTableStateTest's "a column with a + * comparator sorts by it, not by its text", with the real columns. + */ + @Test + fun `airscout sorts numerically`() { + val columns = columns() + assertNotNull(columns.first { it.id == "airscout" }.comparator, "the AP column sorts by its own key") + + val table = DataTableState(columns = columns, rowKey = RowKeys.byValue { it.callSign.orEmpty() }) + table.replaceRows(listOf(stationWithAeroplane("DL13ABC", minutes = 13), stationWithAeroplane("DL9ABC", minutes = 9))) + + table.toggleSort("airscout") + + assertEquals(listOf("DL9ABC", "DL13ABC"), table.visibleRows.map { it.callSign }, + "9 before 13 numerically; by text it would be the other way round") + } + + private fun stationWithAeroplane(call: String, minutes: Int): ChatMember = ChatMember().apply { + setCallSign(call) + airPlaneReflectInfo = AirPlaneReflectionInfo().apply { + risingAirplanes = mutableListOf(AirPlane().apply { arrivingDurationMinutes = minutes; potential = 75 }) + } + } + @Test fun `tropo and score come from outside the station`() { val columns = StationColumns.all( diff --git a/github_docs/de-Changelog.md b/github_docs/de-Changelog.md index a4d04f7f..f59085ff 100644 --- a/github_docs/de-Changelog.md +++ b/github_docs/de-Changelog.md @@ -40,6 +40,8 @@ Mehrere Operateure an einem Rechner können jetzt eigene Rufzeichen, Locators un - **Freigegebene Hintergrundressourcen:** Der ON4KST-Überwachungsthread, der Sked-Erinnerungs-Scheduler, der Reachability-Executor, der PSTRotator-Wiederholungs-Scheduler und der Kachel-Proxy der Karte werden beim Schließen des Chatcontrollers freigegeben. Bisher liefen sie bis zum Programmende weiter. +- **AP-Spalte sortiert numerisch:** Die Spalte **AP [minutes / pot%]** in der Benutzerliste und bei den gerichteten Nachrichten wurde nach ihrem Text sortiert, sodass `13 (75%)` vor `3 (50%)` stand. Jetzt sortiert sie nach den Minuten des ersten Flugzeugs und bei gleichen Minuten nach dem besseren Potenzial; leere und `nil`-Zellen stehen aufsteigend am Ende. Damit ist [Issue #87](https://github.com/praktimarc/kst4contest/issues/87) behoben. + ### Hinweise zur Aktualisierung - Bestehende Installationen werden **nicht** verändert: `preferences.xml` und `praktiKST.db` bleiben genau dort liegen, wo sie sind, und werden zum Profil **Default**. Es wird keine Datei verschoben oder kopiert. diff --git a/github_docs/en-Changelog.md b/github_docs/en-Changelog.md index 3f267d29..4fa4a079 100644 --- a/github_docs/en-Changelog.md +++ b/github_docs/en-Changelog.md @@ -40,6 +40,8 @@ Several operators sharing one computer can now use their own callsigns, locators - **Background resources are released:** the ON4KST supervisor thread, the sked reminder scheduler, the reachability executor, the PSTRotator retry scheduler and the map tile proxy are released when the chat controller is closed. They used to keep running until the program ended. +- **AP column sorts numerically:** the **AP [minutes / pot%]** column in the user list and in the directed messages was sorted by its text, which put `13 (75%)` above `3 (50%)`. It now sorts by the minutes of the first aeroplane and, for equal minutes, by the better potential first; empty and `nil` cells go to the bottom in ascending order. This fixes [Issue #87](https://github.com/praktimarc/kst4contest/issues/87). + ### Upgrade notes - Existing installations are **not** modified: `preferences.xml` and `praktiKST.db` stay exactly where they are and become the **Default** profile. No file is moved or copied.