Skip to content

Commit 5aab043

Browse files
committed
fix a few more minor issues
1 parent 7d151c2 commit 5aab043

9 files changed

Lines changed: 87 additions & 84 deletions

File tree

code/missioneditor/sexp_annotation_model.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,16 @@ SCP_list<int> SexpAnnotationModel::buildPath(int key, const SCP_vector<sexp_tree
170170
return path;
171171

172172
// Walk up to find the root (a node with no parent).
173+
// Guard against cycles with a depth limit based on the tree size.
173174
int root = key;
174-
while (tree_nodes[root].parent >= 0)
175+
int depth = 0;
176+
const int max_depth = static_cast<int>(tree_nodes.size());
177+
while (tree_nodes[root].parent >= 0 && depth < max_depth) {
175178
root = tree_nodes[root].parent;
179+
++depth;
180+
}
181+
if (depth >= max_depth)
182+
return path; // cycle detected
176183

177184
// Find the event index whose formula matches this root.
178185
int event_idx = -1;

code/missioneditor/sexp_tree_actions.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,8 @@ int SexpTreeActions::add_default_operator(int op_index, int argnum)
431431
} else if (Sexp_variables[sexp_var_index].type & SEXP_VARIABLE_NUMBER) {
432432
type |= SEXPT_NUMBER;
433433
} else {
434-
Int3();
434+
Assertion(false, "Unexpected sexp variable type %d for variable '%s'",
435+
Sexp_variables[sexp_var_index].type, item.text.c_str());
435436
}
436437

437438
char node_text[2 * TOKEN_LENGTH + 2];
@@ -461,7 +462,7 @@ int SexpTreeActions::add_default_operator(int op_index, int argnum)
461462
} else if (temp_type & SEXP_VARIABLE_STRING) {
462463
type = SEXPT_VALID | SEXPT_STRING;
463464
} else {
464-
Int3();
465+
Assertion(false, "Unexpected sexp variable type %d for modify-variable default", temp_type);
465466
}
466467
add_data(item.text.c_str(), type);
467468
}
@@ -482,7 +483,7 @@ int SexpTreeActions::insert_operator(int op, void* root_parent_handle)
482483
const int wrapped_node = _model.item_index;
483484
const int parent_node = _model.tree_nodes[wrapped_node].parent;
484485
const int node_flags = _model.tree_nodes[wrapped_node].flags;
485-
const void* wrapped_handle = _model.tree_nodes[wrapped_node].handle;
486+
void* wrapped_handle = _model.tree_nodes[wrapped_node].handle;
486487

487488
const int node = _model.allocate_node(parent_node, wrapped_node);
488489
_model.set_node(node, (SEXPT_OPERATOR | SEXPT_VALID), Operators[op].text.c_str());
@@ -501,7 +502,7 @@ int SexpTreeActions::insert_operator(int op, void* root_parent_handle)
501502
}
502503

503504
_model.tree_nodes[node].handle =
504-
_ui.ui_insert_item(Operators[op].text.c_str(), NodeImage::OPERATOR, insert_parent, const_cast<void*>(wrapped_handle));
505+
_ui.ui_insert_item(Operators[op].text.c_str(), NodeImage::OPERATOR, insert_parent, wrapped_handle);
505506

506507
_ui.ui_move_branch(wrapped_node, node);
507508
_model.item_index = node;
@@ -571,7 +572,7 @@ void SexpTreeActions::replace_variable_with_type_validation(int var_idx, int cur
571572
} else if (Sexp_variables[var_idx].type & SEXP_VARIABLE_STRING) {
572573
resolved_type = SEXPT_STRING;
573574
} else {
574-
Int3();
575+
Assertion(false, "Unexpected sexp variable type %d for variable index %d", Sexp_variables[var_idx].type, var_idx);
575576
}
576577
} else {
577578
if (resolved_type & SEXPT_NUMBER) {
@@ -739,7 +740,7 @@ void SexpTreeActions::verify_and_fix_arguments(int node)
739740
break;
740741

741742
default:
742-
Int3();
743+
Assertion(false, "Unexpected OPF type %d for modify-variable argument", type);
743744
}
744745
}
745746

code/missioneditor/sexp_tree_model.h

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -349,56 +349,59 @@ struct SexpContextMenuState {
349349
SexpContextMenuState& operator=(const SexpContextMenuState&) = delete;
350350
SexpContextMenuState(SexpContextMenuState&& other) noexcept
351351
{
352-
*this = std::move(other);
352+
swap(other);
353353
}
354354
SexpContextMenuState& operator=(SexpContextMenuState&& other) noexcept
355355
{
356356
if (this != &other) {
357-
cleanup();
358-
359-
is_labeled_root = other.is_labeled_root;
360-
is_root_editable = other.is_root_editable;
361-
can_edit_text = other.can_edit_text;
362-
can_edit_comment = other.can_edit_comment;
363-
can_edit_bg_color = other.can_edit_bg_color;
364-
can_add_variable = other.can_add_variable;
365-
can_modify_variable = other.can_modify_variable;
366-
can_copy = other.can_copy;
367-
can_cut = other.can_cut;
368-
can_paste = other.can_paste;
369-
can_paste_add = other.can_paste_add;
370-
can_delete = other.can_delete;
371-
can_replace_number = other.can_replace_number;
372-
can_replace_string = other.can_replace_string;
373-
can_add_number = other.can_add_number;
374-
can_add_string = other.can_add_string;
375-
add_type = other.add_type;
376-
replace_type = other.replace_type;
377-
insert_opf_type = other.insert_opf_type;
378-
add_count = other.add_count;
379-
replace_count = other.replace_count;
380-
modify_variable = other.modify_variable;
381-
campaign_mode = other.campaign_mode;
382-
add_data_list = other.add_data_list;
383-
add_data_opf_type = other.add_data_opf_type;
384-
replace_data_list = other.replace_data_list;
385-
add_enabled_op_indices = std::move(other.add_enabled_op_indices);
386-
replace_enabled_op_indices = std::move(other.replace_enabled_op_indices);
387-
op_add_enabled = std::move(other.op_add_enabled);
388-
op_replace_enabled = std::move(other.op_replace_enabled);
389-
op_insert_enabled = std::move(other.op_insert_enabled);
390-
replace_variables = std::move(other.replace_variables);
391-
show_container_names = other.show_container_names;
392-
replace_container_names = std::move(other.replace_container_names);
393-
show_container_data = other.show_container_data;
394-
replace_container_data = std::move(other.replace_container_data);
395-
396-
other.add_data_list = nullptr;
397-
other.replace_data_list = nullptr;
357+
SexpContextMenuState tmp;
358+
swap(other); // take other's resources
359+
other.swap(tmp); // give other a clean state (tmp gets our old resources and destroys them)
398360
}
399361
return *this;
400362
}
401363

364+
void swap(SexpContextMenuState& other) noexcept
365+
{
366+
using std::swap;
367+
swap(is_labeled_root, other.is_labeled_root);
368+
swap(is_root_editable, other.is_root_editable);
369+
swap(can_edit_text, other.can_edit_text);
370+
swap(can_edit_comment, other.can_edit_comment);
371+
swap(can_edit_bg_color, other.can_edit_bg_color);
372+
swap(can_add_variable, other.can_add_variable);
373+
swap(can_modify_variable, other.can_modify_variable);
374+
swap(can_copy, other.can_copy);
375+
swap(can_cut, other.can_cut);
376+
swap(can_paste, other.can_paste);
377+
swap(can_paste_add, other.can_paste_add);
378+
swap(can_delete, other.can_delete);
379+
swap(can_replace_number, other.can_replace_number);
380+
swap(can_replace_string, other.can_replace_string);
381+
swap(can_add_number, other.can_add_number);
382+
swap(can_add_string, other.can_add_string);
383+
swap(add_type, other.add_type);
384+
swap(replace_type, other.replace_type);
385+
swap(insert_opf_type, other.insert_opf_type);
386+
swap(add_count, other.add_count);
387+
swap(replace_count, other.replace_count);
388+
swap(modify_variable, other.modify_variable);
389+
swap(campaign_mode, other.campaign_mode);
390+
swap(add_data_list, other.add_data_list);
391+
swap(add_data_opf_type, other.add_data_opf_type);
392+
swap(replace_data_list, other.replace_data_list);
393+
swap(add_enabled_op_indices, other.add_enabled_op_indices);
394+
swap(replace_enabled_op_indices, other.replace_enabled_op_indices);
395+
swap(op_add_enabled, other.op_add_enabled);
396+
swap(op_replace_enabled, other.op_replace_enabled);
397+
swap(op_insert_enabled, other.op_insert_enabled);
398+
swap(replace_variables, other.replace_variables);
399+
swap(show_container_names, other.show_container_names);
400+
swap(replace_container_names, other.replace_container_names);
401+
swap(show_container_data, other.show_container_data);
402+
swap(replace_container_data, other.replace_container_data);
403+
}
404+
402405
// Free the dynamically allocated data lists (safe to call multiple times)
403406
void cleanup() {
404407
if (add_data_list) { add_data_list->destroy(); add_data_list = nullptr; }

fred2/campaigneditordlg.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ int campaign_editor::onRootDeleted(int formula_node)
6969
}
7070
}
7171

72-
Campaign_tree_viewp->delete_link(i);
73-
m_num_links--;
72+
if (i < Total_links) {
73+
Campaign_tree_viewp->delete_link(i);
74+
m_num_links--;
75+
}
7476
return formula_node;
7577
}
7678

qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,6 @@ ShipEditorDialogModel::ShipEditorDialogModel(QObject* parent, EditorViewport* vi
2929
initializeData();
3030
}
3131

32-
void ShipEditorDialogModel::setTreeControls(sexp_tree_view* arrival, sexp_tree_view* departure)
33-
{
34-
_arrival_tree_widget = arrival;
35-
_departure_tree_widget = departure;
36-
}
3732

3833
int ShipEditorDialogModel::tristate_set(int val, int cur_state)
3934
{
@@ -813,20 +808,14 @@ bool ShipEditorDialogModel::update_ship(int ship)
813808
if (Ships[ship].arrival_cue >= 0)
814809
free_sexp2(Ships[ship].arrival_cue);
815810

816-
if (_arrival_tree_dirty && _arrival_tree_widget)
817-
Ships[ship].arrival_cue = _arrival_tree_widget->_model.save_tree();
818-
else
819-
Ships[ship].arrival_cue = _m_arrival_tree_formula;
811+
Ships[ship].arrival_cue = _m_arrival_tree_formula;
820812
}
821813

822814
if (!multi_edit || _m_update_departure) {
823815
if (Ships[ship].departure_cue >= 0)
824816
free_sexp2(Ships[ship].departure_cue);
825817

826-
if (_departure_tree_dirty && _departure_tree_widget)
827-
Ships[ship].departure_cue = _departure_tree_widget->_model.save_tree();
828-
else
829-
Ships[ship].departure_cue = _m_departure_tree_formula;
818+
Ships[ship].departure_cue = _m_departure_tree_formula;
830819
}
831820
Ships[ship].arrival_distance = _m_arrival_dist;
832821
Ships[ship].arrival_delay = _m_arrival_delay;
@@ -1259,8 +1248,9 @@ bool ShipEditorDialogModel::getArrivalCue() const
12591248
return _m_update_arrival;
12601249
}
12611250

1262-
void ShipEditorDialogModel::setArrivalTreeDirty()
1251+
void ShipEditorDialogModel::setArrivalTreeDirty(int formula)
12631252
{
1253+
_m_arrival_tree_formula = formula;
12641254
_arrival_tree_dirty = true;
12651255
_m_update_arrival = true;
12661256
set_modified();
@@ -1331,8 +1321,9 @@ bool ShipEditorDialogModel::getDepartureCue() const
13311321
return _m_update_departure;
13321322
}
13331323

1334-
void ShipEditorDialogModel::setDepartureTreeDirty()
1324+
void ShipEditorDialogModel::setDepartureTreeDirty(int formula)
13351325
{
1326+
_m_departure_tree_formula = formula;
13361327
_departure_tree_dirty = true;
13371328
_m_update_departure = true;
13381329
set_modified();

qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
#include "../AbstractDialogModel.h"
44
#include "mission/util.h"
55
#include "ship/ship.h"
6-
#include "ui/widgets/sexp_tree_view.h"
76

87
namespace fso::fred::dialogs {
98
/**
@@ -18,8 +17,6 @@ class ShipEditorDialogModel : public AbstractDialogModel {
1817
int _m_arrival_tree_formula;
1918
bool _arrival_tree_dirty = false;
2019
bool _departure_tree_dirty = false;
21-
sexp_tree_view* _arrival_tree_widget = nullptr;
22-
sexp_tree_view* _departure_tree_widget = nullptr;
2320
SCP_string _m_ship_name;
2421
SCP_string _m_ship_display_name;
2522
SCP_string _m_cargo1;
@@ -79,7 +76,6 @@ class ShipEditorDialogModel : public AbstractDialogModel {
7976

8077
public:
8178
ShipEditorDialogModel(QObject* parent, EditorViewport* viewport);
82-
void setTreeControls(sexp_tree_view* arrival, sexp_tree_view* departure);
8379
void initializeData();
8480
bool apply() override;
8581
void reject() override;
@@ -149,7 +145,7 @@ class ShipEditorDialogModel : public AbstractDialogModel {
149145
void setArrivalCue(const bool);
150146
bool getArrivalCue() const;
151147

152-
void setArrivalTreeDirty();
148+
void setArrivalTreeDirty(int formula);
153149
int getArrivalFormula() const;
154150

155151
void setNoArrivalWarp(const int);
@@ -169,7 +165,7 @@ class ShipEditorDialogModel : public AbstractDialogModel {
169165
void setDepartureCue(const bool);
170166
bool getDepartureCue() const;
171167

172-
void setDepartureTreeDirty();
168+
void setDepartureTreeDirty(int formula);
173169
int getDepartureFormula() const;
174170
void setNoDepartureWarp(const int);
175171
int getNoDepartureWarp() const;

qtfred/src/ui/dialogs/ShipEditor/ShipEditorDialog.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ ShipEditorDialog::ShipEditorDialog(FredView* parent, EditorViewport* viewport)
2222
{
2323
this->setFocus();
2424
ui->setupUi(this);
25-
_model->setTreeControls(ui->arrivalTree, ui->departureTree);
26-
2725
connect(_model.get(), &AbstractDialogModel::modelChanged, this, [this] { updateUI(false); });
2826
connect(this, &QDialog::accepted, _model.get(), &ShipEditorDialogModel::apply);
2927
connect(viewport->editor, &Editor::currentObjectChanged, this, &ShipEditorDialog::update);
@@ -839,7 +837,7 @@ void ShipEditorDialog::on_noArrivalWarpCheckBox_toggled(bool value)
839837
}
840838
void ShipEditorDialog::on_arrivalTree_modified()
841839
{
842-
_model->setArrivalTreeDirty();
840+
_model->setArrivalTreeDirty(ui->arrivalTree->_model.save_tree());
843841
}
844842
void ShipEditorDialog::on_arrivalTree_helpChanged(const QString& help)
845843
{
@@ -869,7 +867,7 @@ void fred::dialogs::ShipEditorDialog::on_updateDepartureCueCheckBox_toggled(bool
869867
}
870868
void fred::dialogs::ShipEditorDialog::on_departureTree_modified()
871869
{
872-
_model->setDepartureTreeDirty();
870+
_model->setDepartureTreeDirty(ui->departureTree->_model.save_tree());
873871
}
874872
void fred::dialogs::ShipEditorDialog::on_departureTree_helpChanged(const QString& help)
875873
{

qtfred/src/ui/dialogs/ShipEditor/ShipEditorDialog.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "ShipAltShipClass.h"
1313

1414
#include <mission/dialogs/ShipEditor/ShipEditorDialogModel.h>
15+
#include <missioneditor/sexp_tree_model.h>
1516
#include <ui/FredView.h>
1617
#include <QtWidgets/QDialog>
1718

qtfred/src/ui/widgets/sexp_tree_view.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ void sexp_tree_view::ensure_visible(int node) {
478478
void sexp_tree_view::move_branch(int source, int parent) {
479479
if (source != -1) {
480480
_model.move_branch_data(source, parent);
481-
if (parent) {
481+
if (parent != -1) {
482482
move_branch(tree_item_handle(tree_nodes[source]), tree_item_handle(tree_nodes[parent]));
483483
} else {
484484
move_branch(tree_item_handle(tree_nodes[source]));
@@ -845,12 +845,16 @@ void sexp_tree_view::update_help(QTreeWidgetItem* h) {
845845
return;
846846
}
847847

848-
// Validate operator help strings
849-
for (const auto& oper : Operators) {
850-
for (const auto& menu : op_menu) {
851-
if (get_category(oper.value) == menu.id) {
852-
if (!help(oper.value)) {
853-
mprintf(("Allender! If you add new sexp operators, add help for them too! :) Sexp %s has no help.\n", oper.text.c_str()));
848+
// Validate operator help strings (once per session)
849+
static bool help_validated = false;
850+
if (!help_validated) {
851+
help_validated = true;
852+
for (const auto& oper : Operators) {
853+
for (const auto& menu : op_menu) {
854+
if (get_category(oper.value) == menu.id) {
855+
if (!help(oper.value)) {
856+
mprintf(("Allender! If you add new sexp operators, add help for them too! :) Sexp %s has no help.\n", oper.text.c_str()));
857+
}
854858
}
855859
}
856860
}
@@ -1582,9 +1586,9 @@ void sexp_tree_view::handleItemChange(QTreeWidgetItem* item, int /*column*/) {
15821586
}
15831587

15841588
if (result.update_node) {
1589+
_model.apply_label_edit(node, result.resolved_text);
15851590
modified();
15861591
nodeChanged(node);
1587-
_model.apply_label_edit(node, result.resolved_text);
15881592
} else {
15891593
auto len = strlen(tree_nodes[node].text);
15901594
item->setText(0, QString::fromUtf8(tree_nodes[node].text, static_cast<int>(len)));

0 commit comments

Comments
 (0)