Skip to content

QtFRED Prop Dialog Cleanup #7418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MjnMixael wants to merge 1 commit into
base: master
Choose a base branch
from
Open

QtFRED Prop Dialog Cleanup #7418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 70 additions & 94 deletions qtfred/src/mission/dialogs/PropEditorDialogModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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<int> 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<int>(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) {
Expand Down Expand Up @@ -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<int> 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<int>(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<std::pair<SCP_string, size_t>>& PropEditorDialogModel::getFlagLabels() const {
Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions qtfred/src/mission/dialogs/PropEditorDialogModel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::pair<SCP_string, size_t>>& getFlagLabels() const;
const SCP_vector<int>& getFlagState() const;
Expand All @@ -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();
Expand Down
13 changes: 4 additions & 9 deletions qtfred/src/ui/dialogs/PropEditorDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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();
}
}
Expand All @@ -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
Loading