Skip to content

Commit

Permalink
#5643: Extend IConflictResolutionAction to be able to access both (so…
Browse files Browse the repository at this point in the history 
…urce and target) entities.
  • Loading branch information
@codereader
codereader committed Jul 3, 2021
1 parent 2ee17a8 commit 9a8ed7c
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 27 deletions.
7 changes: 5 additions & 2 deletions include/imapmerge.h
View file
Expand Up @@ -117,8 +117,11 @@ class IConflictResolutionAction :
// The action that happened in the target (can be empty)
virtual const IMergeAction::Ptr& getTargetAction() const = 0;

// The affected entity node
virtual const INodePtr& getConflictingEntity() const = 0;
// The source entity node causing the conflict
virtual const INodePtr& getConflictingSourceEntity() const = 0;

// The affected entity node in the target map
virtual const INodePtr& getConflictingTargetEntity() const = 0;

// Whether this action has been resolved at all, and what has been chosen
virtual ResolutionType getResolution() const = 0;
Expand Down
41 changes: 27 additions & 14 deletions libs/scene/merge/MergeAction.h
View file
Expand Up @@ -282,7 +282,8 @@ class ConflictResolutionAction :
{
protected:
ConflictType _conflictType;
INodePtr _conflictingEntity;
INodePtr _conflictingSourceEntity;
INodePtr _conflictingTargetEntity;

// The action the source diff is trying to apply
IMergeAction::Ptr _sourceAction;
Expand All @@ -292,14 +293,17 @@ class ConflictResolutionAction :
ResolutionType _resolution;

protected:
ConflictResolutionAction(ConflictType conflictType, const INodePtr& conflictingEntity, const IMergeAction::Ptr& sourceAction) :
ConflictResolutionAction(conflictType, conflictingEntity, sourceAction, IMergeAction::Ptr())
ConflictResolutionAction(ConflictType conflictType, const INodePtr& conflictingSourceEntity,
const INodePtr& conflictingTargetEntity, const IMergeAction::Ptr& sourceAction) :
ConflictResolutionAction(conflictType, conflictingSourceEntity, conflictingTargetEntity, sourceAction, IMergeAction::Ptr())
{}

ConflictResolutionAction(ConflictType conflictType, const INodePtr& conflictingEntity, const IMergeAction::Ptr& sourceAction, const IMergeAction::Ptr& targetAction) :
ConflictResolutionAction(ConflictType conflictType, const INodePtr& conflictingSourceEntity,
const INodePtr& conflictingTargetEntity, const IMergeAction::Ptr& sourceAction, const IMergeAction::Ptr& targetAction) :
MergeAction(ActionType::ConflictResolution),
_conflictType(conflictType),
_conflictingEntity(conflictingEntity),
_conflictingSourceEntity(conflictingSourceEntity),
_conflictingTargetEntity(conflictingTargetEntity),
_sourceAction(sourceAction),
_targetAction(targetAction),
_resolution(ResolutionType::Unresolved)
Expand All @@ -325,14 +329,20 @@ class ConflictResolutionAction :
return _targetAction;
}

const INodePtr& getConflictingEntity() const override
const INodePtr& getConflictingTargetEntity() const override
{
return _conflictingEntity;
return _conflictingTargetEntity;
}

const INodePtr& getConflictingSourceEntity() const override
{
return _conflictingSourceEntity;
}

INodePtr getAffectedNode() override
{
return getConflictingEntity();
// We don't want to return empty references, so use the source entity if the target entity is no longer here
return _conflictingTargetEntity ? _conflictingTargetEntity : _conflictingSourceEntity;
}

ResolutionType getResolution() const override
Expand Down Expand Up @@ -369,15 +379,17 @@ class EntityConflictResolutionAction :
public ConflictResolutionAction
{
public:
EntityConflictResolutionAction(ConflictType conflictType, const INodePtr& conflictingEntity, const MergeAction::Ptr& sourceAction) :
EntityConflictResolutionAction(conflictType, conflictingEntity, sourceAction, MergeAction::Ptr())
EntityConflictResolutionAction(ConflictType conflictType, const INodePtr& conflictingSourceEntity,
const INodePtr& conflictingTargetEntity, const MergeAction::Ptr& sourceAction) :
EntityConflictResolutionAction(conflictType, conflictingSourceEntity, conflictingTargetEntity, sourceAction, MergeAction::Ptr())
{}

EntityConflictResolutionAction(ConflictType conflictType,
const INodePtr& conflictingEntity,
const INodePtr& conflictingSourceEntity,
const INodePtr& conflictingTargetEntity,
const MergeAction::Ptr& sourceAction,
const MergeAction::Ptr& targetAction) :
ConflictResolutionAction(conflictType, conflictingEntity, sourceAction, targetAction)
ConflictResolutionAction(conflictType, conflictingSourceEntity, conflictingTargetEntity, sourceAction, targetAction)
{}
};

Expand All @@ -387,10 +399,11 @@ class EntityKeyValueConflictResolutionAction :
{
public:
EntityKeyValueConflictResolutionAction(ConflictType conflictType,
const INodePtr& conflictingEntity,
const INodePtr& conflictingSourceEntity,
const INodePtr& conflictingTargetEntity,
const MergeAction::Ptr& sourceAction,
const MergeAction::Ptr& targetAction) :
ConflictResolutionAction(conflictType, conflictingEntity, sourceAction, targetAction)
ConflictResolutionAction(conflictType, conflictingSourceEntity, conflictingTargetEntity, sourceAction, targetAction)
{}
};

Expand Down
15 changes: 15 additions & 0 deletions libs/scene/merge/MergeLib.h
View file
Expand Up @@ -161,6 +161,21 @@ inline std::string getAffectedKeyValue(const IMergeAction::Ptr& action)
return keyValueAction ? keyValueAction->getValue() : std::string();
}

// Resolves the single selected merge conflict by keeping both entity versions.
// The entity in the target map won't receive any changes, whereas the source entity will
// be marked for addition to the target map.
inline void resolveConflictByKeepingBothEntities()
{
auto conflictNode = getSingleSelectedConflictNode();

if (conflictNode->getAffectedNode()->getNodeType() != INode::Type::Entity) return;

conflictNode->foreachMergeAction([&](const IMergeAction::Ptr& action)
{
// TODO
});
}

}

}
9 changes: 6 additions & 3 deletions libs/scene/merge/ThreeWayMergeOperation.cpp
View file
Expand Up @@ -152,7 +152,8 @@ void ThreeWayMergeOperation::processEntityModification(const ComparisonResult::E
// When the user chooses to import the change, it will be an AddEntity action
addAction(std::make_shared<EntityConflictResolutionAction>(
ConflictType::ModificationOfRemovedEntity,
sourceDiff.sourceNode, // the conflicting entity (comes from the source map)
sourceDiff.sourceNode, // the conflicting source entity
INodePtr(), // the target entity (is no longer present)
std::make_shared<AddEntityAction>(sourceDiff.sourceNode, _targetRoot)
));
return;
Expand Down Expand Up @@ -221,7 +222,8 @@ void ThreeWayMergeOperation::processEntityModification(const ComparisonResult::E
// Create a conflict resolution action for this key value change
addAction(std::make_shared<EntityKeyValueConflictResolutionAction>(
conflictType,
targetDiff.sourceNode, // conflicting entity
sourceDiff.sourceNode, // conflicting source entity
targetDiff.sourceNode, // conflicting target entity
createActionForKeyValueDiff(sourceKeyValueDiff, targetDiff.sourceNode), // conflicting source change
createActionForKeyValueDiff(*targetKeyValueDiff, targetDiff.sourceNode) // conflicting target change
));
Expand Down Expand Up @@ -313,7 +315,8 @@ void ThreeWayMergeOperation::compareAndCreateActions()
// Entity has been removed in source, but modified in target, this is a conflict
addAction(std::make_shared<EntityConflictResolutionAction>(
ConflictType::RemovalOfModifiedEntity,
targetDiff->second->sourceNode, // conflicting entity
INodePtr(), // conflicting source entity (is not present anymore)
targetDiff->second->sourceNode, // conflicting target entity
std::make_shared<RemoveEntityAction>(targetDiff->second->sourceNode) // conflicting change
));

Expand Down
4 changes: 2 additions & 2 deletions radiant/ui/merge/MergeControlDialog.cpp
View file
Expand Up @@ -258,9 +258,9 @@ void MergeControlDialog::onResolveReject(wxCommandEvent& ev)

void MergeControlDialog::onResolveKeepBoth(wxCommandEvent& ev)
{
UndoableCommand undo("resolveMergeConflictByKeepingBothChanges");
UndoableCommand undo("resolveMergeConflictByKeepingBothEntities");

// TODO
scene::merge::resolveConflictByKeepingBothEntities();
}

void MergeControlDialog::onJumpToNextConflict(wxCommandEvent& ev)
Expand Down
16 changes: 10 additions & 6 deletions test/MapMerging.cpp
View file
Expand Up @@ -1884,7 +1884,7 @@ TEST_F(ThreeWayMergeTest, MergeEntityNameCollisions)
auto keyValueConflict = findAction<EntityKeyValueConflictResolutionAction>(operation,
[](const std::shared_ptr<EntityKeyValueConflictResolutionAction>& action)
{
return Node_getEntity(action->getConflictingEntity())->getKeyValue("name") == "node_3";
return Node_getEntity(action->getConflictingSourceEntity())->getKeyValue("name") == "node_3";
});
EXPECT_TRUE(keyValueConflict);
EXPECT_EQ(keyValueConflict->getConflictType(), ConflictType::SettingKeyToDifferentValue);
Expand Down Expand Up @@ -1934,7 +1934,7 @@ TEST_F(ThreeWayMergeTest, RemovalOfModifiedEntity)
auto entityConflict = findAction<EntityConflictResolutionAction>(operation,
[](const std::shared_ptr<EntityConflictResolutionAction>& action)
{
return Node_getEntity(action->getConflictingEntity())->getKeyValue("name") == "func_static_8";
return Node_getEntity(action->getConflictingTargetEntity())->getKeyValue("name") == "func_static_8";
});
EXPECT_TRUE(entityConflict) << "Didn't find the conflicting with subject func_static_8";
EXPECT_EQ(entityConflict->getConflictType(), ConflictType::RemovalOfModifiedEntity);
Expand Down Expand Up @@ -1962,7 +1962,8 @@ TEST_F(ThreeWayMergeTest, ModificationOfRemovedEntity)
auto entityConflict = findAction<EntityConflictResolutionAction>(operation,
[](const std::shared_ptr<EntityConflictResolutionAction>& action)
{
return Node_getEntity(action->getConflictingEntity())->getKeyValue("name") == "light_1";
return action->getConflictingSourceEntity() && action->getConflictingTargetEntity() == nullptr &&
Node_getEntity(action->getConflictingSourceEntity())->getKeyValue("name") == "light_1";
});
EXPECT_TRUE(entityConflict) << "Didn't find the conflicting with subject light_1";
EXPECT_EQ(entityConflict->getConflictType(), ConflictType::ModificationOfRemovedEntity);
Expand Down Expand Up @@ -1990,7 +1991,8 @@ TEST_F(ThreeWayMergeTest, SettingKeyValueToConflictingValues)
auto valueConflict = findAction<EntityKeyValueConflictResolutionAction>(operation,
[](const std::shared_ptr<EntityKeyValueConflictResolutionAction>& action)
{
return Node_getEntity(action->getConflictingEntity())->getKeyValue("name") == "func_static_6";
return Node_getEntity(action->getConflictingSourceEntity())->getKeyValue("name") == "func_static_6" &&
Node_getEntity(action->getConflictingTargetEntity())->getKeyValue("name") == "func_static_6";
});
EXPECT_TRUE(valueConflict) << "Didn't find the conflicting with subject func_static_6";
EXPECT_EQ(valueConflict->getConflictType(), ConflictType::SettingKeyToDifferentValue);
Expand Down Expand Up @@ -2031,7 +2033,8 @@ TEST_F(ThreeWayMergeTest, ModificationOfRemovedKeyValue)
[](const std::shared_ptr<EntityKeyValueConflictResolutionAction>& action)
{
return action->getConflictType() == ConflictType::ModificationOfRemovedKeyValue &&
Node_getEntity(action->getConflictingEntity())->getKeyValue("name") == "expandable";
Node_getEntity(action->getConflictingSourceEntity())->getKeyValue("name") == "expandable" &&
Node_getEntity(action->getConflictingTargetEntity())->getKeyValue("name") == "expandable";
});
EXPECT_TRUE(valueConflict) << "Didn't find the conflicting with subject expandable";
EXPECT_EQ(valueConflict->getConflictType(), ConflictType::ModificationOfRemovedKeyValue);
Expand Down Expand Up @@ -2072,7 +2075,8 @@ TEST_F(ThreeWayMergeTest, RemovalOfModifiedKeyValue)
[](const std::shared_ptr<EntityKeyValueConflictResolutionAction>& action)
{
return action->getConflictType() == ConflictType::RemovalOfModifiedKeyValue &&
Node_getEntity(action->getConflictingEntity())->getKeyValue("name") == "expandable";
Node_getEntity(action->getConflictingSourceEntity())->getKeyValue("name") == "expandable" &&
Node_getEntity(action->getConflictingTargetEntity())->getKeyValue("name") == "expandable";
});
EXPECT_TRUE(valueConflict) << "Didn't find the conflicting with subject expandable";
EXPECT_EQ(valueConflict->getConflictType(), ConflictType::RemovalOfModifiedKeyValue);
Expand Down

0 comments on commit 9a8ed7c

Please sign in to comment.