Merge pull request TrenchBroom#4339 from TrenchBroom/remove-BrushFace…

…ReferenceException

Remove BrushFaceReferenceException

common/src/Exceptions.h

@@ -36,12 +36,6 @@ class Exception : public std::exception
   const char* what() const noexcept override;
 };
 
-class BrushFaceReferenceException : public Exception
-{
-public:
-  using Exception::Exception;
-};
-
 class EntityAttributeException : public Exception
 {
 public:

common/src/Model/BrushFaceReference.cpp

@@ -19,36 +19,35 @@
 
 #include "BrushFaceReference.h"
 
-#include "Exceptions.h"
+#include "Error.h"
 #include "Model/Brush.h"
 #include "Model/BrushFace.h"
 #include "Model/BrushNode.h"
 
+#include <kdl/result.h>
+#include <kdl/result_fold.h>
 #include <kdl/vector_utils.h>
 
+#include <vecmath/plane_io.h>
+
 #include <cassert>
 
-namespace TrenchBroom
-{
-namespace Model
+namespace TrenchBroom::Model
 {
 BrushFaceReference::BrushFaceReference(BrushNode* node, const BrushFace& face)
-  : m_node(node)
-  , m_facePlane(face.boundary())
+  : m_node{node}
+  , m_facePlane{face.boundary()}
 {
   assert(m_node != nullptr);
 }
 
-BrushFaceHandle BrushFaceReference::resolve() const
+Result<BrushFaceHandle> BrushFaceReference::resolve() const
 {
   if (const auto faceIndex = m_node->brush().findFace(m_facePlane))
   {
     return BrushFaceHandle(m_node, *faceIndex);
   }
-  else
-  {
-    throw BrushFaceReferenceException();
-  }
+  return Error{"Cannot resolve brush face reference"};
 }
 
 std::vector<BrushFaceReference> createRefs(const std::vector<BrushFaceHandle>& handles)
@@ -58,11 +57,10 @@ std::vector<BrushFaceReference> createRefs(const std::vector<BrushFaceHandle>& h
   });
 }
 
-std::vector<BrushFaceHandle> resolveAllRefs(
+Result<std::vector<BrushFaceHandle>> resolveAllRefs(
   const std::vector<BrushFaceReference>& faceRefs)
 {
-  return kdl::vec_transform(
-    faceRefs, [](const auto& faceRef) { return faceRef.resolve(); });
+  return kdl::fold_results(
+    kdl::vec_transform(faceRefs, [](const auto& faceRef) { return faceRef.resolve(); }));
 }
-} // namespace Model
-} // namespace TrenchBroom
+} // namespace TrenchBroom::Model

common/src/Model/BrushFaceReference.h

@@ -21,14 +21,13 @@
 
 #include "FloatType.h"
 #include "Model/BrushFaceHandle.h"
+#include "Result.h"
 
 #include <vecmath/plane.h>
 
 #include <vector>
 
-namespace TrenchBroom
-{
-namespace Model
+namespace TrenchBroom::Model
 {
 class BrushNode;
 
@@ -57,11 +56,9 @@ class BrushFaceReference
   BrushFaceReference(Model::BrushNode* node, const Model::BrushFace& face);
 
   /**
-   * Resolves the referenced brush face.
-   *
-   * @throws BrushFaceReferenceException if the face cannot be resolved
+   * Resolves the referenced brush face or an error if this reference cannot be resolved.
    */
-  BrushFaceHandle resolve() const;
+  Result<BrushFaceHandle> resolve() const;
 };
 
 /**
@@ -71,12 +68,9 @@ std::vector<BrushFaceReference> createRefs(const std::vector<BrushFaceHandle>& h
 
 /**
  * Returns a vector brush face handles representing the faces to which the given face
- * references are resolved.
- *
- * @throws BrushFAceReferenceException if any of the given face references cannot be
- * resolved
+ * references are resolved or an error if any reference cannot be resolved.
  */
-std::vector<BrushFaceHandle> resolveAllRefs(
+Result<std::vector<BrushFaceHandle>> resolveAllRefs(
   const std::vector<BrushFaceReference>& faceRefs);
-} // namespace Model
-} // namespace TrenchBroom
+
+} // namespace TrenchBroom::Model

common/src/View/SelectionCommand.cpp

@@ -20,6 +20,7 @@
 #include "SelectionCommand.h"
 
 #include "Ensure.h"
+#include "Error.h"
 #include "Macros.h"
 #include "Model/BrushFace.h"
 #include "Model/BrushFaceHandle.h"
@@ -29,28 +30,28 @@
 #include "Model/WorldNode.h"
 #include "View/MapDocumentCommandFacade.h"
 
+#include <kdl/result.h>
 #include <kdl/string_format.h>
 #include <kdl/vector_utils.h>
 
 #include <sstream>
 #include <string>
 
-namespace TrenchBroom
-{
-namespace View
+namespace TrenchBroom::View
 {
+
 std::unique_ptr<SelectionCommand> SelectionCommand::select(
-  const std::vector<Model::Node*>& nodes)
+  std::vector<Model::Node*> nodes)
 {
   return std::make_unique<SelectionCommand>(
-    Action::SelectNodes, nodes, std::vector<Model::BrushFaceHandle>{});
+    Action::SelectNodes, std::move(nodes), std::vector<Model::BrushFaceHandle>{});
 }
 
 std::unique_ptr<SelectionCommand> SelectionCommand::select(
-  const std::vector<Model::BrushFaceHandle>& faces)
+  std::vector<Model::BrushFaceHandle> faces)
 {
   return std::make_unique<SelectionCommand>(
-    Action::SelectFaces, std::vector<Model::Node*>{}, faces);
+    Action::SelectFaces, std::vector<Model::Node*>{}, std::move(faces));
 }
 
 std::unique_ptr<SelectionCommand> SelectionCommand::convertToFaces()
@@ -78,17 +79,17 @@ std::unique_ptr<SelectionCommand> SelectionCommand::selectAllFaces()
 }
 
 std::unique_ptr<SelectionCommand> SelectionCommand::deselect(
-  const std::vector<Model::Node*>& nodes)
+  std::vector<Model::Node*> nodes)
 {
   return std::make_unique<SelectionCommand>(
-    Action::DeselectNodes, nodes, std::vector<Model::BrushFaceHandle>{});
+    Action::DeselectNodes, std::move(nodes), std::vector<Model::BrushFaceHandle>{});
 }
 
 std::unique_ptr<SelectionCommand> SelectionCommand::deselect(
-  const std::vector<Model::BrushFaceHandle>& faces)
+  std::vector<Model::BrushFaceHandle> faces)
 {
   return std::make_unique<SelectionCommand>(
-    Action::DeselectFaces, std::vector<Model::Node*>{}, faces);
+    Action::DeselectFaces, std::vector<Model::Node*>{}, std::move(faces));
 }
 
 std::unique_ptr<SelectionCommand> SelectionCommand::deselectAll()
@@ -101,12 +102,12 @@ std::unique_ptr<SelectionCommand> SelectionCommand::deselectAll()
 
 SelectionCommand::SelectionCommand(
   const Action action,
-  const std::vector<Model::Node*>& nodes,
-  const std::vector<Model::BrushFaceHandle>& faces)
-  : UndoableCommand(makeName(action, nodes.size(), faces.size()), false)
-  , m_action(action)
-  , m_nodes(nodes)
-  , m_faceRefs(Model::createRefs(faces))
+  std::vector<Model::Node*> nodes,
+  std::vector<Model::BrushFaceHandle> faces)
+  : UndoableCommand{makeName(action, nodes.size(), faces.size()), false}
+  , m_action{action}
+  , m_nodes{std::move(nodes)}
+  , m_faceRefs{Model::createRefs(faces)}
 {
 }
 
@@ -160,30 +161,37 @@ std::unique_ptr<CommandResult> SelectionCommand::doPerformDo(
   {
   case Action::SelectNodes:
     document->performSelect(m_nodes);
-    break;
+    return std::make_unique<CommandResult>(true);
   case Action::SelectFaces:
-    document->performSelect(Model::resolveAllRefs(m_faceRefs));
-    break;
+    return std::make_unique<CommandResult>(
+      Model::resolveAllRefs(m_faceRefs)
+        .transform([&](const auto& faceHandles) { document->performSelect(faceHandles); })
+        .transform_error([&](const auto& e) { document->error() << e.msg; })
+        .is_success());
   case Action::SelectAllNodes:
     document->performSelectAllNodes();
-    break;
+    return std::make_unique<CommandResult>(true);
   case Action::SelectAllFaces:
     document->performSelectAllBrushFaces();
-    break;
+    return std::make_unique<CommandResult>(true);
   case Action::ConvertToFaces:
     document->performConvertToBrushFaceSelection();
-    break;
+    return std::make_unique<CommandResult>(true);
   case Action::DeselectNodes:
     document->performDeselect(m_nodes);
-    break;
+    return std::make_unique<CommandResult>(true);
   case Action::DeselectFaces:
-    document->performDeselect(Model::resolveAllRefs(m_faceRefs));
-    break;
+    return std::make_unique<CommandResult>(
+      Model::resolveAllRefs(m_faceRefs)
+        .transform(
+          [&](const auto& faceHandles) { document->performDeselect(faceHandles); })
+        .transform_error([&](const auto& e) { document->error() << e.msg; })
+        .is_success());
   case Action::DeselectAll:
     document->performDeselectAll();
-    break;
+    return std::make_unique<CommandResult>(true);
+    switchDefault();
   }
-  return std::make_unique<CommandResult>(true);
 }
 
 std::unique_ptr<CommandResult> SelectionCommand::doPerformUndo(
@@ -196,9 +204,13 @@ std::unique_ptr<CommandResult> SelectionCommand::doPerformUndo(
   }
   if (!m_previouslySelectedFaceRefs.empty())
   {
-    document->performSelect(Model::resolveAllRefs(m_previouslySelectedFaceRefs));
+    return std::make_unique<CommandResult>(
+      Model::resolveAllRefs(m_previouslySelectedFaceRefs)
+        .transform([&](const auto& faceHandles) { document->performSelect(faceHandles); })
+        .transform_error([&](const auto& e) { document->error() << e.msg; })
+        .is_success());
   }
   return std::make_unique<CommandResult>(true);
 }
-} // namespace View
-} // namespace TrenchBroom
+
+} // namespace TrenchBroom::View

common/src/View/SelectionCommand.h

@@ -28,19 +28,18 @@
 #include <tuple>
 #include <vector>
 
-namespace TrenchBroom
-{
-namespace Model
+namespace TrenchBroom::Model
 {
 class BrushFace;
 class BrushFaceHandle;
 class BrushFaceReference;
 class BrushNode;
 class Node;
-} // namespace Model
+} // namespace TrenchBroom::Model
 
-namespace View
+namespace TrenchBroom::View
 {
+
 class SelectionCommand : public UndoableCommand
 {
 private:
@@ -65,24 +64,23 @@ class SelectionCommand : public UndoableCommand
   std::vector<Model::BrushFaceReference> m_previouslySelectedFaceRefs;
 
 public:
-  static std::unique_ptr<SelectionCommand> select(const std::vector<Model::Node*>& nodes);
+  static std::unique_ptr<SelectionCommand> select(std::vector<Model::Node*> nodes);
   static std::unique_ptr<SelectionCommand> select(
-    const std::vector<Model::BrushFaceHandle>& faces);
+    std::vector<Model::BrushFaceHandle> faces);
 
   static std::unique_ptr<SelectionCommand> convertToFaces();
   static std::unique_ptr<SelectionCommand> selectAllNodes();
   static std::unique_ptr<SelectionCommand> selectAllFaces();
 
+  static std::unique_ptr<SelectionCommand> deselect(std::vector<Model::Node*> nodes);
   static std::unique_ptr<SelectionCommand> deselect(
-    const std::vector<Model::Node*>& nodes);
-  static std::unique_ptr<SelectionCommand> deselect(
-    const std::vector<Model::BrushFaceHandle>& faces);
+    std::vector<Model::BrushFaceHandle> faces);
   static std::unique_ptr<SelectionCommand> deselectAll();
 
   SelectionCommand(
     Action action,
-    const std::vector<Model::Node*>& nodes,
-    const std::vector<Model::BrushFaceHandle>& faces);
+    std::vector<Model::Node*> nodes,
+    std::vector<Model::BrushFaceHandle> faces);
   ~SelectionCommand() override;
 
 private:
@@ -94,5 +92,5 @@ class SelectionCommand : public UndoableCommand
 
   deleteCopyAndMove(SelectionCommand);
 };
-} // namespace View
-} // namespace TrenchBroom
+
+} // namespace TrenchBroom::View