diff --git a/qtfred/src/mission/dialogs/PropEditorDialogModel.cpp b/qtfred/src/mission/dialogs/PropEditorDialogModel.cpp index 653cd6d6b1d..035bcc67814 100644 --- a/qtfred/src/mission/dialogs/PropEditorDialogModel.cpp +++ b/qtfred/src/mission/dialogs/PropEditorDialogModel.cpp @@ -17,54 +17,11 @@ PropEditorDialogModel::PropEditorDialogModel(QObject* parent, EditorViewport* vi } bool PropEditorDialogModel::apply() { - if (!hasValidSelection()) { - return true; - } - - if (!validateData()) { - return false; - } - - for (auto obj_idx : _selectedPropObjects) { - if (!query_valid_object(obj_idx) || Objects[obj_idx].type != OBJ_PROP) { - continue; - } - - auto instance = Objects[obj_idx].instance; - auto prp = prop_id_lookup(instance); - if (prp == nullptr) { - continue; - } - - if (!hasMultipleSelection()) { - strcpy_s(prp->prop_name, _propName.c_str()); - } - - for (size_t i = 0; i < _flagLabels.size(); ++i) { - auto state = _flagState[i]; - if (state == Qt::PartiallyChecked) { - continue; - } - - auto flag_index = _flagLabels[i].second; - if (flag_index >= Num_parse_prop_flags) { - continue; - } - - auto& def = Parse_prop_flags[flag_index]; - if (!stricmp(def.name, "no_collide")) { - Objects[obj_idx].flags.set(Object::Object_Flags::Collides, state != Qt::Checked); - } - } - } - - _editor->missionChanged(); return true; } -void PropEditorDialogModel::reject() { - // no-op -} +void PropEditorDialogModel::reject() {} + void PropEditorDialogModel::initializeData() { _flagLabels.clear(); @@ -109,40 +66,6 @@ void PropEditorDialogModel::initializeData() { _modified = false; } -bool PropEditorDialogModel::validateData() { - _bypass_errors = false; - - if (hasMultipleSelection()) { - // Name is not editable for multi-select and only flags are applied. - return true; - } - - SCP_trim(_propName); - if (_propName.empty()) { - showErrorDialogNoCancel("A prop name cannot be empty."); - return false; - } - - std::unordered_set selected_instances; - for (auto obj_idx : _selectedPropObjects) { - if (query_valid_object(obj_idx) && Objects[obj_idx].type == OBJ_PROP) { - selected_instances.insert(Objects[obj_idx].instance); - } - } - - for (size_t i = 0; i < Props.size(); ++i) { - if (selected_instances.find(static_cast(i)) != selected_instances.end() || !Props[i].has_value()) { - continue; - } - - if (!stricmp(_propName.c_str(), Props[i].value().prop_name)) { - showErrorDialogNoCancel("This prop name is already being used by another prop."); - return false; - } - } - - return true; -} void PropEditorDialogModel::showErrorDialogNoCancel(const SCP_string& message) { if (_bypass_errors) { @@ -254,11 +177,49 @@ const SCP_string& PropEditorDialogModel::getPropName() const { return _propName; } -void PropEditorDialogModel::setPropName(const SCP_string& name) { +bool PropEditorDialogModel::setPropName(const SCP_string& name) { if (hasMultipleSelection()) { - return; + return true; + } + + _bypass_errors = false; + + SCP_string trimmed = name; + SCP_trim(trimmed); + + if (trimmed.empty()) { + showErrorDialogNoCancel("A prop name cannot be empty."); + return false; + } + + std::unordered_set selected_instances; + for (auto obj_idx : _selectedPropObjects) { + if (query_valid_object(obj_idx) && Objects[obj_idx].type == OBJ_PROP) { + selected_instances.insert(Objects[obj_idx].instance); + } + } + + for (size_t i = 0; i < Props.size(); ++i) { + if (selected_instances.find(static_cast(i)) != selected_instances.end() || !Props[i].has_value()) { + continue; + } + if (!stricmp(trimmed.c_str(), Props[i].value().prop_name)) { + showErrorDialogNoCancel("This prop name is already being used by another prop."); + return false; + } + } + + auto obj_idx = _selectedPropObjects.front(); + auto prp = prop_id_lookup(Objects[obj_idx].instance); + if (prp == nullptr) { + return false; } - modify(_propName, name); + + strcpy_s(prp->prop_name, trimmed.c_str()); + _propName = trimmed; + set_modified(); + _editor->missionChanged(); + return true; } const SCP_vector>& PropEditorDialogModel::getFlagLabels() const { @@ -274,11 +235,32 @@ void PropEditorDialogModel::setFlagState(size_t index, int state) { return; } - if (_flagState[index] != state) { - _flagState[index] = state; - set_modified(); - Q_EMIT modelChanged(); + if (_flagState[index] == state) { + return; + } + + _flagState[index] = state; + + if (state == Qt::PartiallyChecked) { + return; + } + + auto flag_index = _flagLabels[index].second; + if (flag_index >= Num_parse_prop_flags) { + return; + } + + auto& def = Parse_prop_flags[flag_index]; + for (auto obj_idx : _selectedPropObjects) { + if (!query_valid_object(obj_idx) || Objects[obj_idx].type != OBJ_PROP) { + continue; + } + if (!stricmp(def.name, "no_collide")) { + Objects[obj_idx].flags.set(Object::Object_Flags::Collides, state != Qt::Checked); + } } + set_modified(); + // Caller is responsible for triggering missionChanged() (deferred to avoid FlagListWidget re-entrancy) } SCP_string PropEditorDialogModel::getLayer() const @@ -317,10 +299,7 @@ void PropEditorDialogModel::selectNextProp() { } return; } - - if (apply()) { - selectPropFromObjectList(GET_NEXT(&Objects[_selectedPropObjects.front()]), true); - } + selectPropFromObjectList(GET_NEXT(&Objects[_selectedPropObjects.front()]), true); } void PropEditorDialogModel::selectPreviousProp() { @@ -330,10 +309,7 @@ void PropEditorDialogModel::selectPreviousProp() { } return; } - - if (apply()) { - selectPropFromObjectList(GET_PREV(&Objects[_selectedPropObjects.front()]), false); - } + selectPropFromObjectList(GET_PREV(&Objects[_selectedPropObjects.front()]), false); } void PropEditorDialogModel::onSelectedObjectChanged(int) { diff --git a/qtfred/src/mission/dialogs/PropEditorDialogModel.h b/qtfred/src/mission/dialogs/PropEditorDialogModel.h index ceb607678ce..b51ec3d55b4 100644 --- a/qtfred/src/mission/dialogs/PropEditorDialogModel.h +++ b/qtfred/src/mission/dialogs/PropEditorDialogModel.h @@ -18,7 +18,7 @@ class PropEditorDialogModel : public AbstractDialogModel { bool hasMultipleSelection() const; static bool hasAnyPropsInMission(); const SCP_string& getPropName() const; - void setPropName(const SCP_string& name); + bool setPropName(const SCP_string& name); const SCP_vector>& getFlagLabels() const; const SCP_vector& getFlagState() const; @@ -41,7 +41,6 @@ class PropEditorDialogModel : public AbstractDialogModel { private: // NOLINT(readability-redundant-access-specifiers) void initializeData(); - bool validateData(); void showErrorDialogNoCancel(const SCP_string& message); void selectPropFromObjectList(object* start, bool forward); void selectFirstPropInMission(); diff --git a/qtfred/src/ui/dialogs/PropEditorDialog.cpp b/qtfred/src/ui/dialogs/PropEditorDialog.cpp index d3cab36b7e3..2e5171f5c19 100644 --- a/qtfred/src/ui/dialogs/PropEditorDialog.cpp +++ b/qtfred/src/ui/dialogs/PropEditorDialog.cpp @@ -35,10 +35,9 @@ PropEditorDialog::PropEditorDialog(FredView* parent, EditorViewport* viewport) } } } - // Applying immediately can re-enter FlagListWidget while it is still processing - // itemChanged, which may invalidate the underlying item/model pointers. - // Queue the apply until the current signal stack unwinds. - QMetaObject::invokeMethod(this, [this]() { _model->apply(); }, Qt::QueuedConnection); + // Defer missionChanged to avoid re-entering FlagListWidget while it processes itemChanged, + // which could invalidate the underlying item/model pointers. + QMetaObject::invokeMethod(this, [this]() { _viewport->editor->missionChanged(); }, Qt::QueuedConnection); }); resize(QDialog::sizeHint()); @@ -90,8 +89,7 @@ void PropEditorDialog::updateUi() { } void PropEditorDialog::on_propNameLineEdit_editingFinished() { - _model->setPropName(ui->propNameLineEdit->text().toUtf8().constData()); - if (!_model->apply()) { + if (!_model->setPropName(ui->propNameLineEdit->text().toUtf8().constData())) { updateUi(); } } @@ -108,9 +106,6 @@ void PropEditorDialog::on_layerCombo_currentIndexChanged(int index) { if (index < 0) return; _model->setLayer(ui->layerCombo->itemData(index).toString().toUtf8().constData()); - if (!_model->apply()) { - updateUi(); - } } } // namespace fso::fred::dialogs