From 36af155e7b9af01af4d8b14ecf48ed998407b949 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Mon, 13 Jul 2026 13:41:17 -0400 Subject: [PATCH] give the "Set as Player Ship" button exclusive semantics in both editors In single-player missions, the "Set as Player Ship" button is supposed to make the selected ship the one and only player start. FRED did this correctly, but qtFRED routed the button through the same setPlayer() toggle used by the multiplayer "Player Ship" checkbox, which only promoted the selected ship and never demoted the previous player start. Since the checkbox is disabled in single player, there was no way to demote the old start, so single-player missions could accumulate multiple player starts. Consolidate the exclusive logic into a shared helper, set_single_player_start() in missioneditor/common.cpp, and call it from both FRED's OnSetAsPlayerShip() and a new qtFRED model method, makeSolePlayerStart(), which the button now uses. This also avoids setPlayer()'s early return on stale dialog state, which could cause the button to silently do nothing. Follow-up to #7600. Co-Authored-By: Claude Fable 5 --- code/missioneditor/common.cpp | 54 +++++++++++++++++-- code/missioneditor/common.h | 5 ++ fred2/shipeditordlg.cpp | 40 ++++---------- .../ShipEditor/ShipEditorDialogModel.cpp | 18 +++++++ .../ShipEditor/ShipEditorDialogModel.h | 1 + .../dialogs/ShipEditor/ShipEditorDialog.cpp | 2 +- 6 files changed, 87 insertions(+), 33 deletions(-) diff --git a/code/missioneditor/common.cpp b/code/missioneditor/common.cpp index e24d8648a35..4262ed3df0f 100644 --- a/code/missioneditor/common.cpp +++ b/code/missioneditor/common.cpp @@ -153,16 +153,64 @@ void ensure_valid_player_start_shipnum() // nothing to do if the current player start is still valid if (Player_start_shipnum >= 0 && Player_start_shipnum < MAX_SHIPS && Ships[Player_start_shipnum].objnum >= 0 - && Objects[Ships[Player_start_shipnum].objnum].type == OBJ_START) { + && Objects[Ships[Player_start_shipnum].objnum].type == OBJ_START) + { return; } // otherwise repoint to the first remaining player start, or -1 if there are none Player_start_shipnum = -1; - for (auto *objp : list_range(&obj_used_list)) { - if (objp->type == OBJ_START) { + for (auto *objp : list_range(&obj_used_list)) + { + if (objp->type == OBJ_START) + { Player_start_shipnum = objp->instance; break; } } } + +bool set_single_player_start(int objnum) +{ + if (objnum < 0 || objnum >= MAX_OBJECTS + || (Objects[objnum].type != OBJ_SHIP && Objects[objnum].type != OBJ_START)) + { + Assertion(false, "set_single_player_start() called for object %d, which is not a ship", objnum); + return false; + } + + bool changed = false; + + for (auto *objp : list_range(&obj_used_list)) + { + if (objp->type != OBJ_SHIP && objp->type != OBJ_START) + continue; + + if (OBJ_INDEX(objp) == objnum) + { + // set as player ship + if (objp->type != OBJ_START) + { + objp->type = OBJ_START; + Player_starts++; + changed = true; + } + objp->flags.set(Object::Object_Flags::Player_ship); + } + else + { + // set as regular ship + if (objp->type == OBJ_START) + { + objp->type = OBJ_SHIP; + Player_starts--; + changed = true; + } + objp->flags.remove(Object::Object_Flags::Player_ship); + } + } + + ensure_valid_player_start_shipnum(); + + return changed; +} diff --git a/code/missioneditor/common.h b/code/missioneditor/common.h index 34e5c22e849..ad1e42c4798 100644 --- a/code/missioneditor/common.h +++ b/code/missioneditor/common.h @@ -53,3 +53,8 @@ void generate_weaponry_usage_list_wing(int wing_num, int* arr); // first remaining player start in the mission (or -1 if there are none). Call this after // changing a ship to or from an OBJ_START via demotion, deletion, etc. void ensure_valid_player_start_shipnum(); + +// Make the given object the sole player start, per single-player semantics: promote it to +// OBJ_START, demote every other player start to OBJ_SHIP, adjust Player_starts to match, +// and repoint Player_start_shipnum. Returns true if anything changed. +bool set_single_player_start(int objnum); diff --git a/fred2/shipeditordlg.cpp b/fred2/shipeditordlg.cpp index 602117d4ce1..7f19a854152 100644 --- a/fred2/shipeditordlg.cpp +++ b/fred2/shipeditordlg.cpp @@ -2431,40 +2431,22 @@ void CShipEditorDlg::OnSetAsPlayerShip() return; } - // since this is single player, clear all player ships and set only this one - object *objp = GET_FIRST(&obj_used_list); - while (objp != END_OF_LIST(&obj_used_list)) + // find the selected ship; there should only be one + object *marked_objp = nullptr; + for (auto objp: list_range(&obj_used_list)) { - if ((objp->type == OBJ_SHIP) || (objp->type == OBJ_START)) + if (((objp->type == OBJ_SHIP) || (objp->type == OBJ_START)) && objp->flags[Object::Object_Flags::Marked]) { - if (objp->flags[Object::Object_Flags::Marked]) // there should only be one selected ship - { - // set as player ship - if (objp->type != OBJ_START) - { - Player_starts++; - set_modified(); - } - objp->type = OBJ_START; - objp->flags.set(Object::Object_Flags::Player_ship); - } - else - { - // set as regular ship - if (objp->type == OBJ_START) - { - Player_starts--; - set_modified(); - } - objp->type = OBJ_SHIP; - objp->flags.remove(Object::Object_Flags::Player_ship); - } + marked_objp = objp; + break; } - objp = GET_NEXT(objp); } + if (marked_objp == nullptr) + return; - // fix up Player_start_shipnum if it became invalid - ensure_valid_player_start_shipnum(); + // since this is single player, clear all player ships and set only this one + if (set_single_player_start(OBJ_INDEX(marked_objp))) + set_modified(); // finally set editor dialog m_player_ship.SetCheck(1); diff --git a/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp b/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp index bbd2a12c646..efb6593b64e 100644 --- a/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp +++ b/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp @@ -1162,6 +1162,24 @@ bool ShipEditorDialogModel::getPlayer() const return _isPlayerShip; } +void ShipEditorDialogModel::makeSolePlayerStart() +{ + // the ship being edited, which is either a player start or a regular ship + int shipnum = (_playerShipIndex >= 0) ? _playerShipIndex : _singleShip; + if (shipnum < 0) + return; + + // since this is single player, clear all player ships and set only this one + if (set_single_player_start(Ships[shipnum].objnum)) + { + setModified(); + _editor->missionChanged(); + } + + _isPlayerShip = true; + modelChanged(); +} + void ShipEditorDialogModel::setRespawn(const int value) { if (_respawnPriority == value) diff --git a/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.h b/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.h index 74e6fa0ac61..c319036d150 100644 --- a/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.h +++ b/qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.h @@ -59,6 +59,7 @@ class ShipEditorDialogModel : public AbstractDialogModel { void setPlayer(bool isPlayer); bool getPlayer() const; + void makeSolePlayerStart(); void setRespawn(int respawn); int getRespawn() const; diff --git a/qtfred/src/ui/dialogs/ShipEditor/ShipEditorDialog.cpp b/qtfred/src/ui/dialogs/ShipEditor/ShipEditorDialog.cpp index 9d132838908..76b169192a4 100644 --- a/qtfred/src/ui/dialogs/ShipEditor/ShipEditorDialog.cpp +++ b/qtfred/src/ui/dialogs/ShipEditor/ShipEditorDialog.cpp @@ -641,7 +641,7 @@ void ShipEditorDialog::on_textureReplacementButton_clicked() void ShipEditorDialog::on_playerShipButton_clicked() { - _model->setPlayer(true); + _model->makeSolePlayerStart(); } void ShipEditorDialog::on_altShipClassButton_clicked() {