Skip to content

Commit

Permalink
Don't use result::visit in client code
Browse files Browse the repository at this point in the history
  • Loading branch information
kduske committed Feb 1, 2023
1 parent b38c236 commit 3c42415
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 94 deletions.
16 changes: 8 additions & 8 deletions common/src/IO/MapReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -648,14 +648,14 @@ static std::vector<std::optional<NodeInfo>> createNodesFromObjectInfos(
assert(createNodeResult.has_value());

return std::move(*createNodeResult)
.visit(kdl::overload(
[&](NodeInfo&& nodeInfo) -> std::optional<NodeInfo> {
return std::move(nodeInfo);
},
[&](const NodeError& e) -> std::optional<NodeInfo> {
status.error(e.line, e.msg);
return std::nullopt;
}));
.and_then([&](NodeInfo&& nodeInfo) -> std::optional<NodeInfo> {
return std::move(nodeInfo);
})
.or_else([&](const NodeError& e) -> std::optional<NodeInfo> {
status.error(e.line, e.msg);
return std::nullopt;
})
.value();
});
}

Expand Down
13 changes: 6 additions & 7 deletions common/src/Model/GameImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,12 @@ std::unique_ptr<WorldNode> GameImpl::doNewMap(
const auto builder =
Model::BrushBuilder{worldNode->mapFormat(), worldBounds, defaultFaceAttribs()};
builder.createCuboid({128.0, 128.0, 32.0}, Model::BrushFaceAttributes::NoTextureName)
.visit(kdl::overload(
[&](Brush&& b) {
worldNode->defaultLayer()->addChild(new BrushNode{std::move(b)});
},
[&](const Model::BrushError e) {
logger.error() << "Could not create default brush: " << e;
}));
.and_then([&](Brush&& b) {
worldNode->defaultLayer()->addChild(new BrushNode{std::move(b)});
})
.or_else([&](const Model::BrushError e) {
logger.error() << "Could not create default brush: " << e;
});

return worldNode;
}
Expand Down
12 changes: 6 additions & 6 deletions common/src/View/MapDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3665,12 +3665,12 @@ bool MapDocument::extrudeBrushes(

return brush
.moveBoundary(m_worldBounds, *faceIndex, delta, pref(Preferences::TextureLock))
.visit(kdl::overload(
[&]() { return m_worldBounds.contains(brush.bounds()); },
[&](const Model::BrushError e) {
error() << "Could not resize brush: " << e;
return false;
}));
.and_then([&]() { return m_worldBounds.contains(brush.bounds()); })
.or_else([&](const Model::BrushError e) {
error() << "Could not resize brush: " << e;
return false;
})
.value();
},
[](Model::BezierPatch&) { return true; }));
}
Expand Down
5 changes: 2 additions & 3 deletions common/test/src/Model/tst_Brush.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,8 @@ static bool canMoveBoundary(
const vm::vec3& delta)
{
return brush.moveBoundary(worldBounds, faceIndex, delta, false)
.visit(kdl::overload(
[&]() { return worldBounds.contains(brush.bounds()); },
[](const BrushError) { return false; }));
.and_then([&]() { return worldBounds.contains(brush.bounds()); })
.value_or(false);
}

TEST_CASE("BrushTest.constructBrushWithFaces")
Expand Down
89 changes: 37 additions & 52 deletions common/test/src/Model/tst_GroupNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,18 +188,16 @@ TEST_CASE("GroupNodeTest.updateLinkedGroups")

SECTION("Target group list is empty")
{
const auto updateResult = updateLinkedGroups(groupNode, {}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) { CHECK(r.empty()); },
[](const auto&) { FAIL(); }));
updateLinkedGroups(groupNode, {}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) { CHECK(r.empty()); })
.or_else([](const auto&) { FAIL(); });
}

SECTION("Target group list contains only source group")
{
const auto updateResult = updateLinkedGroups(groupNode, {&groupNode}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) { CHECK(r.empty()); },
[](const auto&) { FAIL(); }));
updateLinkedGroups(groupNode, {&groupNode}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) { CHECK(r.empty()); })
.or_else([](const auto&) { FAIL(); });
}

SECTION("Update a single target group")
Expand All @@ -223,10 +221,8 @@ TEST_CASE("GroupNodeTest.updateLinkedGroups")
*entityNode, vm::translation_matrix(vm::vec3(0.0, 0.0, 3.0)), worldBounds);
REQUIRE(entityNode->entity().origin() == vm::vec3(1.0, 0.0, 3.0));

const auto updateResult =
updateLinkedGroups(groupNode, {groupNodeClone.get()}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) {
updateLinkedGroups(groupNode, {groupNodeClone.get()}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) {
CHECK(r.size() == 1u);

const auto& p = r.front();
Expand All @@ -239,8 +235,8 @@ TEST_CASE("GroupNodeTest.updateLinkedGroups")
CHECK(newEntityNode != nullptr);

CHECK(newEntityNode->entity().origin() == vm::vec3(1.0, 2.0, 3.0));
},
[](const auto&) { FAIL(); }));
})
.or_else([](const auto&) { FAIL(); });
}
}

Expand Down Expand Up @@ -278,10 +274,8 @@ TEST_CASE("GroupNodeTest.updateNestedLinkedGroups")
innerGroupNodeClone->group().transformation()
== vm::translation_matrix(vm::vec3(0.0, 2.0, 0.0)));

const auto updateResult =
updateLinkedGroups(*innerGroupNode, {innerGroupNodeClone.get()}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) {
updateLinkedGroups(*innerGroupNode, {innerGroupNodeClone.get()}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) {
CHECK(r.size() == 1u);

const auto& p = r.front();
Expand All @@ -294,8 +288,8 @@ TEST_CASE("GroupNodeTest.updateNestedLinkedGroups")
CHECK(newEntityNode != nullptr);

CHECK(newEntityNode->entity().origin() == vm::vec3(0.0, 2.0, 0.0));
},
[](const auto&) { FAIL(); }));
})
.or_else([](const auto&) { FAIL(); });
}

SECTION("Transforming the inner group node's entity and updating the linked group")
Expand All @@ -311,10 +305,8 @@ TEST_CASE("GroupNodeTest.updateNestedLinkedGroups")
innerGroupNodeClone->group().transformation()
== vm::translation_matrix(vm::vec3(0.0, 2.0, 0.0)));

const auto updateResult =
updateLinkedGroups(*innerGroupNode, {innerGroupNodeClone.get()}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) {
updateLinkedGroups(*innerGroupNode, {innerGroupNodeClone.get()}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) {
CHECK(r.size() == 1u);

const auto& p = r.front();
Expand All @@ -327,8 +319,8 @@ TEST_CASE("GroupNodeTest.updateNestedLinkedGroups")
CHECK(newEntityNode != nullptr);

CHECK(newEntityNode->entity().origin() == vm::vec3(1.0, 2.0, 0.0));
},
[](const auto&) { FAIL(); }));
})
.or_else([](const auto&) { FAIL(); });
}
}

Expand Down Expand Up @@ -382,10 +374,8 @@ TEST_CASE("GroupNodeTest.updateLinkedGroupsRecursively")
dynamic_cast<EntityNode*>(innerGroupNodeClone->children().front());
REQUIRE(innerGroupEntityNodeClone != nullptr);

const auto updateResult =
updateLinkedGroups(outerGroupNode, {outerGroupNodeClone.get()}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) {
updateLinkedGroups(outerGroupNode, {outerGroupNodeClone.get()}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) {
REQUIRE(r.size() == 1u);
const auto& [groupNodeToUpdate, newChildren] = r.front();

Expand All @@ -401,8 +391,8 @@ TEST_CASE("GroupNodeTest.updateLinkedGroupsRecursively")
dynamic_cast<EntityNode*>(newInnerGroupNodeClone->children().front());
CHECK(newInnerGroupEntityNodeClone != nullptr);
CHECK(newInnerGroupEntityNodeClone->entity() == innerGroupEntityNode->entity());
},
[](const auto&) { FAIL(); }));
})
.or_else([](const auto&) { FAIL(); });
}

TEST_CASE("GroupNodeTest.updateLinkedGroupsExceedsWorldBounds")
Expand All @@ -428,14 +418,13 @@ TEST_CASE("GroupNodeTest.updateLinkedGroupsExceedsWorldBounds")
*entityNode, vm::translation_matrix(vm::vec3(1.0, 0.0, 0.0)), worldBounds);
REQUIRE(entityNode->entity().origin() == vm::vec3(1.0, 0.0, 0.0));

const auto updateResult =
updateLinkedGroups(groupNode, {groupNodeClone.get()}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult&) { FAIL(); },
[](const BrushError&) { FAIL(); },
[](const UpdateLinkedGroupsError& e) {
CHECK(e == UpdateLinkedGroupsError::UpdateExceedsWorldBounds);
}));
updateLinkedGroups(groupNode, {groupNodeClone.get()}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult&) { FAIL(); })
.or_else(kdl::overload(
[](const BrushError&) { FAIL(); },
[](const UpdateLinkedGroupsError& e) {
CHECK(e == UpdateLinkedGroupsError::UpdateExceedsWorldBounds);
}));
}

static void setGroupName(GroupNode& groupNode, const std::string& name)
Expand Down Expand Up @@ -477,19 +466,17 @@ TEST_CASE("GroupNodeTest.updateLinkedGroupsAndPreserveNestedGroupNames")
"Updating outerGroupNode retains the names of its linked group and the nested linked "
"group")
{
const auto updateResult =
updateLinkedGroups(outerGroupNode, {outerGroupNodeClone.get()}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) {
updateLinkedGroups(outerGroupNode, {outerGroupNodeClone.get()}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) {
REQUIRE(r.size() == 1u);

const auto& [groupNodeToUpdate, newChildren] = r.front();
REQUIRE(groupNodeToUpdate == outerGroupNodeClone.get());

const auto* innerReplacement = static_cast<GroupNode*>(newChildren.front().get());
CHECK(innerReplacement->name() == innerGroupNodeNestedClone->name());
},
[](const auto&) { FAIL(); }));
})
.or_else([](const auto&) { FAIL(); });
}
}

Expand Down Expand Up @@ -615,10 +602,8 @@ TEST_CASE("GroupNodeTest.updateLinkedGroupsAndPreserveEntityProperties")
// lambda can't capture structured bindings
const auto expectedTargetProperties = expectedProperties;

const auto updateResult =
updateLinkedGroups(sourceGroupNode, {targetGroupNode.get()}, worldBounds);
updateResult.visit(kdl::overload(
[&](const UpdateLinkedGroupsResult& r) {
updateLinkedGroups(sourceGroupNode, {targetGroupNode.get()}, worldBounds)
.and_then([&](const UpdateLinkedGroupsResult& r) {
REQUIRE(r.size() == 1u);
const auto& p = r.front();

Expand All @@ -634,8 +619,8 @@ TEST_CASE("GroupNodeTest.updateLinkedGroupsAndPreserveEntityProperties")
CHECK_THAT(
newEntityNode->entity().protectedProperties(),
Catch::UnorderedEquals(targetEntityNode->entity().protectedProperties()));
},
[](const auto&) { FAIL(); }));
})
.or_else([](const auto&) { FAIL(); });
}
} // namespace Model
} // namespace TrenchBroom
24 changes: 6 additions & 18 deletions common/test/src/tst_Preferences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,15 +400,9 @@ TEST_CASE("PreferencesTest.readV2")
CHECK(parseV2SettingsFromJSON(QByteArray(R"({"foo": "bar"})")).is_success());
CHECK(parseV2SettingsFromJSON(QByteArray("{}")).is_success());

const ReadPreferencesResult v2 =
readV2SettingsFromPath("fixture/test/preferences-v2.json");
CHECK(v2.is_success());
v2.visit(kdl::overload(
[](const std::map<IO::Path, QJsonValue>& prefs) { testV2Prefs(prefs); },
[](const PreferenceErrors::NoFilePresent&) { FAIL_CHECK(); },
[](const PreferenceErrors::JsonParseError&) { FAIL_CHECK(); },
[](const PreferenceErrors::FileAccessError&) { FAIL_CHECK(); },
[](const PreferenceErrors::LockFileError&) { FAIL_CHECK(); }));
readV2SettingsFromPath("fixture/test/preferences-v2.json")
.and_then([](const std::map<IO::Path, QJsonValue>& prefs) { testV2Prefs(prefs); })
.or_else([](const auto&) { FAIL_CHECK(); });
}

TEST_CASE("PreferencesTest.testWriteReadV2")
Expand All @@ -418,15 +412,9 @@ TEST_CASE("PreferencesTest.testWriteReadV2")
const std::map<IO::Path, QJsonValue> v2 = migrateV1ToV2(v1);

const QByteArray v2Serialized = writeV2SettingsToJSON(v2);
const auto v2Deserialized = parseV2SettingsFromJSON(v2Serialized);

CHECK(v2Deserialized.is_success());
v2Deserialized.visit(kdl::overload(
[&](const std::map<IO::Path, QJsonValue>& prefs) { CHECK(v2 == prefs); },
[](const PreferenceErrors::NoFilePresent&) { FAIL_CHECK(); },
[](const PreferenceErrors::JsonParseError&) { FAIL_CHECK(); },
[](const PreferenceErrors::FileAccessError&) { FAIL_CHECK(); },
[](const PreferenceErrors::LockFileError&) { FAIL_CHECK(); }));
parseV2SettingsFromJSON(v2Serialized)
.and_then([&](const std::map<IO::Path, QJsonValue>& prefs) { CHECK(v2 == prefs); })
.or_else([](const auto&) { FAIL_CHECK(); });
}

/**
Expand Down

0 comments on commit 3c42415

Please sign in to comment.