MapFindFeature: Exclude hidden/protected objects (GH-2371)

The 'Find next' search failed if the object query returned an object
with either a hidden or protected symbol since these kind of objects
can't be added to an object selection. This change excludes such
objects from the result set. A test is added.

---------

Co-authored-by: Kai Pastor <[email protected]>

Author: dl3sdo

src/gui/map/map_find_feature.cpp

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2017-2024 Kai Pastor
+ *    Copyright 2017-2020, 2024, 2025 Kai Pastor
  *
  *    This file is part of OpenOrienteering.
  *
@@ -22,8 +22,8 @@
 
 #include <functional>
 
-#include <QAction>
 #include <QAbstractButton>
+#include <QAction>
 #include <QDialog>
 #include <QDialogButtonBox>
 #include <QGridLayout>
@@ -35,7 +35,9 @@
 
 #include "core/map.h"
 #include "core/map_part.h"
+#include "core/objects/object.h"
 #include "core/objects/object_query.h"
+#include "core/symbols/symbol.h"
 #include "gui/main_window.h"
 #include "gui/util_gui.h"
 #include "gui/map/map_editor.h"
@@ -44,7 +46,17 @@
 
 namespace OpenOrienteering {
 
-class Object;
+namespace {
+
+// Returns true if an object can be added to the selection.
+bool isSelectable(const Object* object)
+{
+	const auto* symbol = object ? object->getSymbol() : nullptr;
+	return symbol && !symbol->isHidden() && !symbol->isProtected();
+}
+
+}  // namespace
+
 
 MapFindFeature::MapFindFeature(MapEditorController& controller)
 : QObject{nullptr}
@@ -117,7 +129,7 @@ void MapFindFeature::showDialog()
 
 		auto button_box = new QDialogButtonBox(QDialogButtonBox::Close | QDialogButtonBox::Help);
 		connect(button_box, &QDialogButtonBox::rejected, &*find_dialog, &QDialog::hide);
-		connect(button_box->button(QDialogButtonBox::Help), &QPushButton::clicked, this, &MapFindFeature::showHelp);
+		connect(button_box, &QDialogButtonBox::helpRequested, this, &MapFindFeature::showHelp);
 
 		editor_stack = new QStackedLayout();
 		editor_stack->addWidget(text_edit);
@@ -166,52 +178,54 @@ ObjectQuery MapFindFeature::makeQuery() const
 			query = tag_selector->makeQuery();
 		}
 	}
+	if (!query)
+	{
+		controller.getMap()->clearObjectSelection(true);
+		controller.getWindow()->showStatusBarMessage(OpenOrienteering::TagSelectWidget::tr("Invalid query"), 2000);
+	}
 	return query;
 }
 
 
 void MapFindFeature::findNext()
 {
-	auto map = controller.getMap();
-	auto first_object = map->getFirstSelectedObject();
+	if (auto query = makeQuery())
+		findNextMatchingObject(controller, query);
+}
+
+// static
+void MapFindFeature::findNextMatchingObject(MapEditorController& controller, const ObjectQuery& query)
+{
+	auto* map = controller.getMap();
+	
+	Object* first_match = nullptr;  // the first match in all objects
+	Object* pivot_object = map->getFirstSelectedObject();
+	Object* next_match = nullptr;   // the next match after pivot_object
 	map->clearObjectSelection(false);
 
-	Object* next_object = nullptr;
-	auto query = makeQuery();
-	if (!query)
-	{
-		if (auto window = controller.getWindow())
-			window->showStatusBarMessage(OpenOrienteering::TagSelectWidget::tr("Invalid query"), 2000);
-		return;
-	}
+	auto search = [&](Object* object) {
+		if (next_match)
+			return;
 
-	auto search = [&first_object, &next_object, &query](Object* object) {
-		if (!next_object)
+		bool after_pivot = (pivot_object == nullptr);
+		if (object == pivot_object)
+			pivot_object = nullptr;
+		
+		if (isSelectable(object) && query(object))
 		{
-			if (first_object)
-			{
-				if (object == first_object)
-					first_object = nullptr;
-			}
-			else if (query(object))
-			{
-				next_object = object;
-			}
+			if (after_pivot)
+				next_match = object;
+			else if (!first_match)
+				first_match = object;
 		}
 	};
 
-	// Start from selected object
 	map->getCurrentPart()->applyOnAllObjects(search);
-	if (!next_object)
-	{
-		// Start from first object
-		first_object = nullptr;
-		map->getCurrentPart()->applyOnAllObjects(search);
-	}
+	if (!next_match)
+		next_match = first_match;
+	if (next_match)
+		map->addObjectToSelection(next_match, false);
 
-	map->clearObjectSelection(false);
-	if (next_object)
-		map->addObjectToSelection(next_object, false);
 	map->emitSelectionChanged();
 	map->ensureVisibilityOfSelectedObjects(Map::FullVisibility);
 
@@ -221,20 +235,22 @@ void MapFindFeature::findNext()
 
 
 void MapFindFeature::findAll()
+{
+	if (auto query = makeQuery())
+		findAllMatchingObjects(controller, query);
+}
+
+// static
+void MapFindFeature::findAllMatchingObjects(MapEditorController& controller, const ObjectQuery& query)
 {
 	auto map = controller.getMap();
 	map->clearObjectSelection(false);
 
-	auto query = makeQuery();
-	if (!query)
-	{
-		controller.getWindow()->showStatusBarMessage(OpenOrienteering::TagSelectWidget::tr("Invalid query"), 2000);
-		return;
-	}
-	
 	map->getCurrentPart()->applyOnMatchingObjects([map](Object* object) {
-		map->addObjectToSelection(object, false);
+		if (isSelectable(object))
+			map->addObjectToSelection(object, false);
 	}, std::cref(query));
+	
 	map->emitSelectionChanged();
 	map->ensureVisibilityOfSelectedObjects(Map::FullVisibility);
 	controller.getWindow()->showStatusBarMessage(OpenOrienteering::TagSelectWidget::tr("%n object(s) selected", nullptr, map->getNumSelectedObjects()), 2000);
@@ -244,15 +260,12 @@ void MapFindFeature::findAll()
 }
 
 
-
 void MapFindFeature::showHelp() const
 {
 	Util::showHelp(controller.getWindow(), "find_objects.html");
 }
 
 
-
-// slot
 void MapFindFeature::tagSelectorToggled(bool active)
 {
 	editor_stack->setCurrentIndex(active ? 1 : 0);

src/gui/map/map_find_feature.h

@@ -1,5 +1,5 @@
 /*
- *    Copyright 2017 Kai Pastor
+ *    Copyright 2017-2019, 2025 Kai Pastor
  *
  *    This file is part of OpenOrienteering.
  *
@@ -37,7 +37,6 @@ class MapEditorController;
 class ObjectQuery;
 class TagSelectWidget;
 
-
 /**
  * Provides an interactive feature for finding objects in the map.
  * 
@@ -48,7 +47,6 @@ class TagSelectWidget;
 class MapFindFeature : public QObject
 {
 	Q_OBJECT
-	
 public:
 	MapFindFeature(MapEditorController& controller);
 
@@ -60,6 +58,10 @@ class MapFindFeature : public QObject
 
 	QAction* findNextAction() { return find_next_action; }
 
+	static void findNextMatchingObject(MapEditorController& controller, const ObjectQuery& query);
+	
+	static void findAllMatchingObjects(MapEditorController& controller, const ObjectQuery& query);
+	
 private:
 	void showDialog();
 
@@ -73,6 +75,7 @@ class MapFindFeature : public QObject
 
 	void tagSelectorToggled(bool active);
 
+	
 	MapEditorController& controller;
 	QPointer<QDialog> find_dialog;           // child of controller's window
 	QStackedLayout* editor_stack = nullptr;  // child of find_dialog
@@ -88,4 +91,4 @@ class MapFindFeature : public QObject
 
 }  // namespace OpenOrienteering
 
-#endif
+#endif // OPENORIENTEERING_MAP_FIND_FEATURE_H

test/tools_t.cpp

@@ -1,6 +1,7 @@
 /*
  *    Copyright 2012, 2013 Thomas Schöps
- *    Copyright 2015-2020 Kai Pastor
+ *    Copyright 2015-2020, 2025 Kai Pastor
+ *    Copyright 2025 Matthias Kühlewein
  *
  *    This file is part of OpenOrienteering.
  *
@@ -35,10 +36,13 @@
 #include "core/map_color.h"
 #include "core/map_coord.h"
 #include "core/objects/object.h"
+#include "core/objects/object_query.h"
 #include "core/symbols/line_symbol.h"
+#include "core/symbols/point_symbol.h"
 #include "global.h"
 #include "gui/main_window.h"
 #include "gui/map/map_editor.h"
+#include "gui/map/map_find_feature.h"
 #include "gui/map/map_widget.h"
 #include "templates/paint_on_template_feature.h"
 #include "tools/edit_point_tool.h"
@@ -161,7 +165,7 @@ void TestMapEditor::simulateDrag(const QPointF& start_pos, const QPointF& end_po
 }
 
 
-// ### TestTools ###
+// ### ToolsTest ###
 
 void ToolsTest::initTestCase()
 {
@@ -229,14 +233,68 @@ void ToolsTest::paintOnTemplateFeature()
 }
 
 
+void ToolsTest::testFindObjects()
+{
+	auto* map = new Map;
+	{
+		auto* normal_point_symbol = new PointSymbol();
+		map->addSymbol(normal_point_symbol, 0);
+		
+		auto* hidden_point_symbol = new PointSymbol();
+		hidden_point_symbol->setHidden(true);
+		map->addSymbol(hidden_point_symbol, 1);
+		
+		auto* protected_point_symbol = new PointSymbol();
+		protected_point_symbol->setProtected(true);
+		map->addSymbol(protected_point_symbol, 2);
+		
+		auto add_object = [map](Symbol* symbol, const char* label) {
+			auto* object = new PointObject(symbol);
+			object->setTag(QLatin1String("match"), QLatin1String(label));
+			map->addObject(object);
+		};
+		add_object(normal_point_symbol, "yes");     // expected match
+		add_object(normal_point_symbol, "no");
+		add_object(normal_point_symbol, "yes");     // expected match
+		add_object(hidden_point_symbol, "yes");
+		add_object(normal_point_symbol, "yes");     // expected match
+		add_object(protected_point_symbol, "yes");
+	}
+	
+	TestMapEditor editor(map);  // taking ownership
+	
+	ObjectQuery query {QLatin1String("match"), ObjectQuery::OperatorIs, QLatin1String("yes")};
+	QVERIFY(query);
+	
+	MapFindFeature::findAllMatchingObjects(*editor.editor, query);
+	QCOMPARE(map->getNumSelectedObjects(), 3);
+	
+	MapFindFeature::findNextMatchingObject(*editor.editor, query);
+	QCOMPARE(map->getNumSelectedObjects(), 1);
+	auto* first_match = map->getFirstSelectedObject();
+	
+	MapFindFeature::findNextMatchingObject(*editor.editor, query);
+	QCOMPARE(map->getNumSelectedObjects(), 1);
+	QVERIFY(map->getFirstSelectedObject() != first_match);
+	
+	MapFindFeature::findNextMatchingObject(*editor.editor, query);
+	QCOMPARE(map->getNumSelectedObjects(), 1);
+	QVERIFY(map->getFirstSelectedObject() != first_match);
+	
+	MapFindFeature::findNextMatchingObject(*editor.editor, query);
+	QCOMPARE(map->getNumSelectedObjects(), 1);
+	QVERIFY(map->getFirstSelectedObject() == first_match);
+}
+
+
 /*
  * We select a non-standard QPA because we don't need a real GUI window.
  * 
  * Normally, the "offscreen" plugin would be the correct one.
  * However, it bails out with a QFontDatabase error (cf. QTBUG-33674)
  */
 namespace  {
-	auto Q_DECL_UNUSED qpa_selected = qputenv("QT_QPA_PLATFORM", "minimal");  // clazy:exclude=non-pod-global-static
+	auto const Q_DECL_UNUSED qpa_selected = qputenv("QT_QPA_PLATFORM", "minimal");  // clazy:exclude=non-pod-global-static
 }
 
 

test/tools_t.h

@@ -1,6 +1,6 @@
 /*
  *    Copyright 2012, 2013 Thomas Schöps
- *    Copyright 2017 Kai Pastor
+ *    Copyright 2017, 2020, 2025 Kai Pastor
  *
  *    This file is part of OpenOrienteering.
  *
@@ -36,6 +36,8 @@ private slots:
 	void editTool();
 
 	void paintOnTemplateFeature();
+	
+	void testFindObjects();
 };
 
 #endif