From 32b379308225c05a92d666d7768a72fd6c9a6672 Mon Sep 17 00:00:00 2001 From: zhexu14 <64713351+zhexu14@users.noreply.github.com> Date: Wed, 11 Oct 2023 12:05:57 +1100 Subject: [PATCH] Fix issues with waypoint editing. fix a number of regressions in the flight waypoint list by changing the indexing and finding a work-around to blocking of signals This PR addresses some (but not all) recently reported issues with the waypoints screen reported in Issue #3188 . This PR was tested by: - Changing the waypoint altitude and confirming it shows up correctly when reloading the waypoint list window and on the map - Adding a waypoint and confirming that it shows up immediately and persists on reload - Deleting a waypoint (except the first waypoint) and confirming that it is removed immediately and persists on reload, Known issues: first waypoint (typically hold) cannot be deleted -- still looking into this one. Fixes https://github.com/dcs-liberation/dcs_liberation/issues/3188. --- changelog.md | 2 + .../flight/waypoints/QFlightWaypointList.py | 80 +++++++++---------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/changelog.md b/changelog.md index 47943571..b0c0a60e 100644 --- a/changelog.md +++ b/changelog.md @@ -43,6 +43,8 @@ Saves from 8.x are not compatible with 9.0.0. * **[UI]** Fixed UI bug where altering an "ahead of package" TOT offset would change the offset back to a "behind pacakge" offset. * **[UI]** Fixed bug where changing TOT offsets could result in flight startup times that are in the past. * **[UI]** Fixed odd spacing of the finance window when there were not enough items to fill the page. +* **[UI]** Fixed regression where waypoint altitude changes in the waypoint list screen are applied to the wrong waypoint. +* **[UI]** Fixed regression where waypoint additions in custom flight plans are not reflected until the window is reloaded. # 8.1.0 diff --git a/qt_ui/windows/mission/flight/waypoints/QFlightWaypointList.py b/qt_ui/windows/mission/flight/waypoints/QFlightWaypointList.py index b64ae30e..a8266e7a 100644 --- a/qt_ui/windows/mission/flight/waypoints/QFlightWaypointList.py +++ b/qt_ui/windows/mission/flight/waypoints/QFlightWaypointList.py @@ -53,46 +53,40 @@ class QFlightWaypointList(QTableView): self.setItemDelegateForColumn(1, self.altitude_editor_delegate) def update_list(self) -> None: - # ignore signals when updating list so on_changed does not fire - self.model.blockSignals(True) - try: - # We need to keep just the row and rebuild the index later because the - # QModelIndex will not be valid after the model is cleared. - current_index = self.currentIndex().row() - self.model.clear() + # We need to keep just the row and rebuild the index later because the + # QModelIndex will not be valid after the model is cleared. + current_index = self.currentIndex().row() + self.model.clear() - self.model.setHorizontalHeaderLabels(HEADER_LABELS) + self.model.setHorizontalHeaderLabels(HEADER_LABELS) - waypoints = self.flight.flight_plan.waypoints - # Why [1:]? Qt starts indexing at 1 rather than 0, whereas DCS numbers - # waypoints starting with 0, and for whatever reason Qt crashes whenever I - # set the vertical labels manually. - # - # Starting with the second waypoint is a bit of a hack, but it's also the - # historical behavior anyway. This view used to have waypoints starting at 1 - # and just didn't show the departure waypoint because the departure waypoint - # wasn't actually part of the flight plan tracked by Liberation. That - # changed at some point, so now we need to skip it manually to preserve that - # behavior. - # - # It really ought to show the departure waypoint and start indexing at 0, - # but since this all pending a move to React anyway, it's not worth fighting - # the Qt crashes for now. - # - # https://github.com/dcs-liberation/dcs_liberation/issues/3037 - for row, waypoint in enumerate(waypoints[1:]): - self._add_waypoint_row(row, self.flight, waypoint) - self.selectionModel().setCurrentIndex( - self.model.index(current_index, 0), QItemSelectionModel.Select - ) - self.resizeColumnsToContents() - total_column_width = self.verticalHeader().width() + self.lineWidth() - for i in range(0, self.model.columnCount()): - total_column_width += self.columnWidth(i) + self.lineWidth() - self.setFixedWidth(total_column_width) - finally: - # stop ignoring signals - self.model.blockSignals(False) + waypoints = self.flight.flight_plan.waypoints + # Why [1:]? Qt starts indexing at 1 rather than 0, whereas DCS numbers + # waypoints starting with 0, and for whatever reason Qt crashes whenever I + # set the vertical labels manually. + # + # Starting with the second waypoint is a bit of a hack, but it's also the + # historical behavior anyway. This view used to have waypoints starting at 1 + # and just didn't show the departure waypoint because the departure waypoint + # wasn't actually part of the flight plan tracked by Liberation. That + # changed at some point, so now we need to skip it manually to preserve that + # behavior. + # + # It really ought to show the departure waypoint and start indexing at 0, + # but since this all pending a move to React anyway, it's not worth fighting + # the Qt crashes for now. + # + # https://github.com/dcs-liberation/dcs_liberation/issues/3037 + for row, waypoint in enumerate(waypoints[1:]): + self._add_waypoint_row(row, self.flight, waypoint) + self.selectionModel().setCurrentIndex( + self.model.index(current_index, 0), QItemSelectionModel.Select + ) + self.resizeColumnsToContents() + total_column_width = self.verticalHeader().width() + self.lineWidth() + for i in range(0, self.model.columnCount()): + total_column_width += self.columnWidth(i) + self.lineWidth() + self.setFixedWidth(total_column_width) def _add_waypoint_row( self, row: int, flight: Flight, waypoint: FlightWaypoint @@ -118,9 +112,13 @@ class QFlightWaypointList(QTableView): def on_changed(self) -> None: for i in range(self.model.rowCount()): - altitude = self.model.item(i, 1).text() - altitude_feet = float(altitude) - self.flight.flight_plan.waypoints[i].alt = Distance.from_feet(altitude_feet) + if self.model.item(i, 1) is not None: + altitude = self.model.item(i, 1).text() + altitude_feet = float(altitude) + # update waypoint index i+1 as rows are 1-indexed + self.flight.flight_plan.waypoints[i + 1].alt = Distance.from_feet( + altitude_feet + ) def tot_text(self, flight: Flight, waypoint: FlightWaypoint) -> str: if waypoint.waypoint_type == FlightWaypointType.TAKEOFF: