Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions code/missioneditor/sexp_tree_model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
// Used by both FRED2 (MFC) and QtFRED (Qt) sexp tree implementations.

constexpr int TREE_NODE_INCREMENT = 100;
constexpr int MIN_SEXP_SEARCH_COST = 10;

// -----------------------------------------------------------------------
// sexp_list_item implementation
Expand Down Expand Up @@ -952,9 +953,8 @@ SCP_string SexpTreeModel::match_closest_operator(const SCP_string& str, int node
opf = query_operator_argument_type(op, arg_num);

// find the best operator
int best = sexp_match_closest_operator(str, opf);
int best = sexp_match_closest_operator(str, opf, MIN_SEXP_SEARCH_COST);
if (best < 0) {
Warning(LOCATION, "Unable to find an operator match for string '%s' and argument type %d", str.c_str(), opf);
return str;
}
return Operators[best].text;
Expand Down
9 changes: 6 additions & 3 deletions code/parse/sexp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35315,12 +35315,15 @@ bool sexp_query_type_match(int opf, int opr)
* Finds the operator that is the best textual match for the input string, given the required OPF type. For equal matches,
* the alphabetically earliest operator is returned.
*
* min defaults to SCP_string::npos, when specified to another value, this function will not always return a match
*
* Note: Returns the operator index, not the operator value.
*/
int sexp_match_closest_operator(const SCP_string &str, int opf)
int sexp_match_closest_operator(const SCP_string &str, int opf, size_t min)
{
// Cyborg - This bool setup helps with readability
bool return_any = (min == SCP_string::npos);
int best = -1;
size_t min = SCP_string::npos;

for (int op_index : Sorted_operator_indexes)
{
Expand All @@ -35330,7 +35333,7 @@ int sexp_match_closest_operator(const SCP_string &str, int opf)
if (sexp_query_type_match(opf, opr))
{
size_t cost = stringcost(op_text, str, Max_operator_length, stringcost_tolower_equal);
if (best < 0 || cost < min)
if (cost < min || (return_any && best == -1) )
{
min = cost;
best = op_index;
Expand Down
2 changes: 1 addition & 1 deletion code/parse/sexp.h
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,7 @@ extern std::pair<int, sexp_src> query_referenced_in_sexp(sexp_ref_type type, con
extern void stuff_sexp_text_string(SCP_string &dest, int node, int mode);
extern int build_sexp_string(SCP_string &accumulator, int cur_node, int level, int mode);
extern bool sexp_query_type_match(int opf, int opr);
extern int sexp_match_closest_operator(const SCP_string &str, int opf);
extern int sexp_match_closest_operator(const SCP_string &str, int opf, size_t min = SCP_string::npos);
extern bool sexp_recoverable_error(int num);
extern const char *sexp_error_message(int num);
extern int count_free_sexp_nodes();
Expand Down
36 changes: 28 additions & 8 deletions qtfred/src/ui/widgets/sexp_tree_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ sexp_tree_view::sexp_tree_view(QWidget* parent) : QTreeWidget(parent), _actions(
installShortcut(QKeySequence::Delete,
[](const SexpContextMenuState& s) { return s.can_delete; },
[this]() { deleteActionHandler(); });

// This keyboard shortcut does not need as much state checking because the helper
// handles security checks and what to call, and any node should work here.
auto* shortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_S), this);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question.. did you verify this doesn't fall through to the editor and call a save action? I don't think it will but just want to be sure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still saves the mission after the dialog is closed.

shortcut->setContext(Qt::WidgetShortcut);
connect(shortcut, &QShortcut::activated, this, &sexp_tree_view::editActionHandlerHelper);
}


Expand Down Expand Up @@ -1143,8 +1149,9 @@ std::unique_ptr<QMenu> sexp_tree_view::buildContextMenu(QTreeWidgetItem* h) {
popup_menu->addSeparator();

auto replace_op_menu = popup_menu->addMenu(tr("Replace Operator"));

auto replace_data_menu = popup_menu->addMenu(tr("Replace Data"));

auto replace_number_act =
replace_data_menu->addAction(tr("Number"), this, [this]() { replaceNumberDataHandler(); });
replace_number_act->setEnabled(false);
Expand All @@ -1153,6 +1160,7 @@ std::unique_ptr<QMenu> sexp_tree_view::buildContextMenu(QTreeWidgetItem* h) {
replace_string_act->setEnabled(false);
replace_data_menu->addSeparator();

popup_menu->addAction(tr("Search for Replacement"), QKeySequence(Qt::CTRL | Qt::Key_S), this, [this]() { editActionHandlerHelper(); });
popup_menu->addSection("Variables");

auto modify_variable_act = popup_menu->addAction(tr("Add/Modify Variable"), this, [this]() {
Expand Down Expand Up @@ -1608,11 +1616,20 @@ void sexp_tree_view::startOperatorQuickSearch(QTreeWidgetItem* item, const QStri
void sexp_tree_view::filterOperatorPopup(const QString& text)
{
_opList->clear();
SCP_set<QString> found_list;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable? Alternatively if you used it for what it seems like was intended you can change this whole lookup from O(n2) into just O(n), I think.

for (const auto& s : _opAll) {
    if (s.startsWith(text, Qt::CaseInsensitive)) {
        _opList->addItem(s);
        found_list.insert(s);
    }
}
for (const auto& s : _opAll) {
    if (s.contains(text, Qt::CaseInsensitive) && found_list.count(s) == 0)
        _opList->addItem(s);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. Again, I think my mental state was a little frazzled. Not that we need a ton of speed here, but I added the check before comparing the text.


if (text.isEmpty()) {
_opList->addItems(_opAll);
} else {
for (const auto& s : _opAll) {
if (s.contains(text, Qt::CaseInsensitive))
if (s.startsWith(text, Qt::CaseInsensitive)){
_opList->addItem(s);
found_list.insert(s);
}
}

for (const auto& s : _opAll) {
if (found_list.insert(s).second && s.contains(text, Qt::CaseInsensitive))
_opList->addItem(s);
}
}
Expand Down Expand Up @@ -1987,13 +2004,16 @@ void sexp_tree_view::handleDoubleClick() {

// Helper function for making sure that search and edit are initiated consistently
void sexp_tree_view::editActionHandlerHelper(){
item_index = get_node(currentItem());

if (_model.compute_context_menu_state().can_edit_text) {
editDataActionHandler();
} else {
openNodeEditor(currentItem());
auto item = currentItem();

// Just to be extra safe, early exit when an item is not selected
if (_currently_editing || _opPopupActive || item == nullptr || !_interface) {
return;
}

item_index = get_node(item);
// No longer gated to just certain types.
openNodeEditor(item);
}

// Public entry point for deleting the currently selected item. Simply delegates to deleteActionHandler().
Expand Down
Loading