Skip to content

Commit

Permalink
#5643: Ensure that adding func_* entities from the source scene keeps…
Browse files Browse the repository at this point in the history 
… the "model"=="name" equality intact.

Move some classes to scenelib.
  • Loading branch information
@codereader
codereader committed Jul 3, 2021
1 parent 9a8ed7c commit c34710e
Show file tree
Hide file tree
Showing 18 changed files with 212 additions and 123 deletions.
3 changes: 3 additions & 0 deletions include/imapmerge.h
View file
Expand Up @@ -145,6 +145,9 @@ class IMergeOperation
// Whether this operation has any actions to perform
virtual bool hasActions() = 0;

// Adds a new action to this operation
virtual void addAction(const IMergeAction::Ptr& action) = 0;

// Invokes the given functor for each action in this operation
virtual void foreachAction(const std::function<void(const IMergeAction::Ptr&)>& visitor) = 0;

Expand Down
2 changes: 1 addition & 1 deletion install/ui/mergecontroldialog.fbp
View file
Expand Up @@ -1864,7 +1864,7 @@
</object>
<object class="sizeritem" expanded="0">
<property name="border">6</property>
<property name="flag">wxALIGN_CENTER_VERTICAL|wxRIGHT</property>
<property name="flag">wxALIGN_CENTER_VERTICAL|wxALIGN_RIGHT|wxRIGHT</property>
<property name="proportion">0</property>
<object class="wxStaticText" expanded="0">
<property name="BottomDockable">1</property>
Expand Down
5 changes: 3 additions & 2 deletions install/ui/mergecontroldialog.xrc
View file
Expand Up @@ -386,7 +386,7 @@
</object>
<object class="sizeritem">
<option>0</option>
<flag>wxALIGN_CENTER_VERTICAL|wxRIGHT</flag>
<flag>wxALIGN_CENTER_VERTICAL|wxALIGN_RIGHT|wxRIGHT</flag>
<border>6</border>
<object class="wxStaticText" name="UnresolvedConflicts">
<font>
Expand Down Expand Up @@ -516,7 +516,8 @@
<flag>wxLEFT</flag>
<border>6</border>
<object class="wxButton" name="ResolveKeepBothButton">
<label>Keep Both Entities</label>
<tooltip>Keeps both versions of this entity</tooltip>
<label>Keep both Entities</label>
<default>0</default>
<markup>0</markup>
<bitmap />
Expand Down
1 change: 1 addition & 0 deletions libs/scene/CMakeLists.txt
View file
Expand Up @@ -6,6 +6,7 @@ add_library(scenegraph
Node.cpp
merge/MergeOperation.cpp
merge/MergeOperationBase.cpp
merge/MergeActionNode.cpp
merge/GraphComparer.cpp
merge/ThreeWayMergeOperation.cpp
SelectableNode.cpp
Expand Down
26 changes: 25 additions & 1 deletion libs/scene/merge/MergeAction.h
View file
Expand Up @@ -113,12 +113,16 @@ class AddCloneToParentAction :
INodePtr _parent;
INodePtr _cloneToBeInserted;

// For func_* based entities, the model key needs to be the same as the name
bool _modelIsEqualToName;

protected:
// Will add the given node to the parent when applyChanges() is called
AddCloneToParentAction(const INodePtr& node, const INodePtr& parent, ActionType type) :
MergeAction(type),
_node(node),
_parent(parent)
_parent(parent),
_modelIsEqualToName(false)
{
assert(_node);
assert(Node_getCloneable(node));
Expand All @@ -139,6 +143,10 @@ class AddCloneToParentAction :
{
child->moveToLayer(activeLayer); return true;
});

auto* entity = Node_getEntity(_cloneToBeInserted);

_modelIsEqualToName = entity && entity->getKeyValue("name") == entity->getKeyValue("model");
}

public:
Expand All @@ -147,6 +155,22 @@ class AddCloneToParentAction :
if (!isActive()) return;

addNodeToContainer(_cloneToBeInserted, _parent);

// Check if we need to synchronise the model and name key values
if (_modelIsEqualToName)
{
auto* entity = Node_getEntity(_cloneToBeInserted);

if (entity)
{
auto curName = entity->getKeyValue("name");

if (curName != entity->getKeyValue("model"))
{
entity->setKeyValue("model", curName);
}
}
}
}

const INodePtr& getParent() const
Expand Down
92 changes: 46 additions & 46 deletions radiantcore/map/MergeActionNode.cpp → libs/scene/merge/MergeActionNode.cpp
View file
@@ -1,6 +1,6 @@
#include "MergeActionNode.h"

namespace map
namespace scene
{

MergeActionNodeBase::MergeActionNodeBase() :
Expand All @@ -12,7 +12,7 @@ void MergeActionNodeBase::prepareForMerge()
_syncActionStatus = false;
}

scene::INodePtr MergeActionNodeBase::getAffectedNode()
INodePtr MergeActionNodeBase::getAffectedNode()
{
return _affectedNode;
}
Expand All @@ -22,11 +22,11 @@ void MergeActionNodeBase::clear()
_affectedNode.reset();
}

void MergeActionNodeBase::onInsertIntoScene(scene::IMapRootNode& rootNode)
void MergeActionNodeBase::onInsertIntoScene(IMapRootNode& rootNode)
{
if (_syncActionStatus)
{
foreachMergeAction([&](const scene::merge::IMergeAction::Ptr& action)
foreachMergeAction([&](const merge::IMergeAction::Ptr& action)
{
action->activate();
});
Expand All @@ -37,22 +37,22 @@ void MergeActionNodeBase::onInsertIntoScene(scene::IMapRootNode& rootNode)
SelectableNode::onInsertIntoScene(rootNode);
}

void MergeActionNodeBase::onRemoveFromScene(scene::IMapRootNode& rootNode)
void MergeActionNodeBase::onRemoveFromScene(IMapRootNode& rootNode)
{
SelectableNode::onRemoveFromScene(rootNode);

unhideAffectedNodes();

if (_syncActionStatus)
{
foreachMergeAction([&](const scene::merge::IMergeAction::Ptr& action)
foreachMergeAction([&](const merge::IMergeAction::Ptr& action)
{
// Removing an unresolved conflict action from the scene implies rejecting the source change
auto conflictAction = std::dynamic_pointer_cast<scene::merge::IConflictResolutionAction>(action);
auto conflictAction = std::dynamic_pointer_cast<merge::IConflictResolutionAction>(action);

if (conflictAction && conflictAction->getResolution() == scene::merge::ResolutionType::Unresolved)
if (conflictAction && conflictAction->getResolution() == merge::ResolutionType::Unresolved)
{
conflictAction->setResolution(scene::merge::ResolutionType::RejectSourceChange);
conflictAction->setResolution(merge::ResolutionType::RejectSourceChange);
}

// Removing any action from the scene means to deactivate it
Expand All @@ -61,9 +61,9 @@ void MergeActionNodeBase::onRemoveFromScene(scene::IMapRootNode& rootNode)
}
}

scene::INode::Type MergeActionNodeBase::getNodeType() const
INode::Type MergeActionNodeBase::getNodeType() const
{
return scene::INode::Type::MergeAction;
return INode::Type::MergeAction;
}

bool MergeActionNodeBase::supportsStateFlag(unsigned int state) const
Expand All @@ -85,7 +85,7 @@ void MergeActionNodeBase::renderSolid(RenderableCollector& collector, const Volu
{
_affectedNode->viewChanged();
_affectedNode->renderSolid(collector, volume);
_affectedNode->foreachNode([&](const scene::INodePtr& child)
_affectedNode->foreachNode([&](const INodePtr& child)
{
child->viewChanged();
child->renderSolid(collector, volume);
Expand All @@ -97,7 +97,7 @@ void MergeActionNodeBase::renderWireframe(RenderableCollector& collector, const
{
_affectedNode->viewChanged();
_affectedNode->renderWireframe(collector, volume);
_affectedNode->foreachNode([&](const scene::INodePtr& child)
_affectedNode->foreachNode([&](const INodePtr& child)
{
child->viewChanged();
child->renderWireframe(collector, volume);
Expand All @@ -114,14 +114,14 @@ void MergeActionNodeBase::testSelect(Selector& selector, SelectionTest& test)
{
testSelectNode(_affectedNode, selector, test);

_affectedNode->foreachNode([&](const scene::INodePtr& child)
_affectedNode->foreachNode([&](const INodePtr& child)
{
testSelectNode(child, selector, test);
return true;
});
}

void MergeActionNodeBase::testSelectNode(const scene::INodePtr& node, Selector& selector, SelectionTest& test)
void MergeActionNodeBase::testSelectNode(const INodePtr& node, Selector& selector, SelectionTest& test)
{
auto selectionTestable = std::dynamic_pointer_cast<SelectionTestable>(node);

Expand All @@ -141,7 +141,7 @@ void MergeActionNodeBase::hideAffectedNodes()
// Hide the affected node itself, we're doing the rendering ourselves, recursively
_affectedNode->enable(Node::eExcluded);

_affectedNode->foreachNode([&](const scene::INodePtr& child)
_affectedNode->foreachNode([&](const INodePtr& child)
{
child->enable(Node::eExcluded);
return true;
Expand All @@ -153,7 +153,7 @@ void MergeActionNodeBase::unhideAffectedNodes()
// Release the excluded state of the contained nodes
_affectedNode->disable(Node::eExcluded);

_affectedNode->foreachNode([&](const scene::INodePtr& child)
_affectedNode->foreachNode([&](const INodePtr& child)
{
child->disable(Node::eExcluded);
return true;
Expand All @@ -162,38 +162,38 @@ void MergeActionNodeBase::unhideAffectedNodes()

// ------------ KeyValueMergeActionNode ----------------------------

KeyValueMergeActionNode::KeyValueMergeActionNode(const std::vector<scene::merge::IMergeAction::Ptr>& actions) :
KeyValueMergeActionNode::KeyValueMergeActionNode(const std::vector<merge::IMergeAction::Ptr>& actions) :
_actions(actions)
{
assert(!_actions.empty());

_affectedNode = _actions.front()->getAffectedNode();
assert(std::find_if(_actions.begin(), _actions.end(),
[&](const scene::merge::IMergeAction::Ptr& action) { return action->getAffectedNode() != _affectedNode; }) == _actions.end());
[&](const merge::IMergeAction::Ptr& action) { return action->getAffectedNode() != _affectedNode; }) == _actions.end());
}

void KeyValueMergeActionNode::clear()
{
_actions.clear();
}

scene::merge::ActionType KeyValueMergeActionNode::getActionType() const
merge::ActionType KeyValueMergeActionNode::getActionType() const
{
// We report the change key value type since we're doing all kinds of key value changes,
// unless we have an unresolved conflict in our collection
auto activeConflict = std::find_if(_actions.begin(), _actions.end(), [&](const scene::merge::IMergeAction::Ptr& action)
auto activeConflict = std::find_if(_actions.begin(), _actions.end(), [&](const merge::IMergeAction::Ptr& action)
{
auto conflict = std::dynamic_pointer_cast<scene::merge::IConflictResolutionAction>(action);
auto conflict = std::dynamic_pointer_cast<merge::IConflictResolutionAction>(action);

return conflict && conflict->isActive() && conflict->getResolution() == scene::merge::ResolutionType::Unresolved;
return conflict && conflict->isActive() && conflict->getResolution() == merge::ResolutionType::Unresolved;
});

if (activeConflict != _actions.end())
{
return scene::merge::ActionType::ConflictResolution;
return merge::ActionType::ConflictResolution;
}

return !_actions.empty() ? scene::merge::ActionType::ChangeKeyValue : scene::merge::ActionType::NoAction;
return !_actions.empty() ? merge::ActionType::ChangeKeyValue : merge::ActionType::NoAction;
}

std::size_t KeyValueMergeActionNode::getMergeActionCount()
Expand All @@ -211,7 +211,7 @@ bool KeyValueMergeActionNode::hasActiveActions()
return false;
}

void KeyValueMergeActionNode::foreachMergeAction(const std::function<void(const scene::merge::IMergeAction::Ptr&)>& functor)
void KeyValueMergeActionNode::foreachMergeAction(const std::function<void(const merge::IMergeAction::Ptr&)>& functor)
{
for (const auto& action : _actions)
{
Expand All @@ -221,13 +221,13 @@ void KeyValueMergeActionNode::foreachMergeAction(const std::function<void(const

// RegularMergeActionNode

RegularMergeActionNode::RegularMergeActionNode(const scene::merge::IMergeAction::Ptr& action) :
RegularMergeActionNode::RegularMergeActionNode(const merge::IMergeAction::Ptr& action) :
_action(action)
{
_affectedNode = _action->getAffectedNode();
}

void RegularMergeActionNode::onInsertIntoScene(scene::IMapRootNode& rootNode)
void RegularMergeActionNode::onInsertIntoScene(IMapRootNode& rootNode)
{
// Add the nodes that are missing in this scene, for preview purposes
addPreviewNodeForAddAction();
Expand All @@ -236,7 +236,7 @@ void RegularMergeActionNode::onInsertIntoScene(scene::IMapRootNode& rootNode)
MergeActionNodeBase::onInsertIntoScene(rootNode);
}

void RegularMergeActionNode::onRemoveFromScene(scene::IMapRootNode& rootNode)
void RegularMergeActionNode::onRemoveFromScene(IMapRootNode& rootNode)
{
MergeActionNodeBase::onRemoveFromScene(rootNode);

Expand All @@ -248,33 +248,33 @@ void RegularMergeActionNode::clear()
_action.reset();
}

scene::merge::ActionType RegularMergeActionNode::getActionType() const
merge::ActionType RegularMergeActionNode::getActionType() const
{
if (!_action) return scene::merge::ActionType::NoAction;
if (!_action) return merge::ActionType::NoAction;

if (_action->getType() == scene::merge::ActionType::ConflictResolution)
if (_action->getType() == merge::ActionType::ConflictResolution)
{
auto conflictAction = std::dynamic_pointer_cast<scene::merge::IConflictResolutionAction>(_action);
auto conflictAction = std::dynamic_pointer_cast<merge::IConflictResolutionAction>(_action);
assert(conflictAction);

// Determine how this node should be rendered (unresolved conflict, or the type of the change that was accepted)
switch (conflictAction->getResolution())
{
case scene::merge::ResolutionType::Unresolved:
return scene::merge::ActionType::ConflictResolution;
case merge::ResolutionType::Unresolved:
return merge::ActionType::ConflictResolution;

case scene::merge::ResolutionType::ApplySourceChange: // render using the accepted action type
case merge::ResolutionType::ApplySourceChange: // render using the accepted action type
return conflictAction->getSourceAction()->getType();

case scene::merge::ResolutionType::RejectSourceChange:
return scene::merge::ActionType::NoAction;
case merge::ResolutionType::RejectSourceChange:
return merge::ActionType::NoAction;
}
}

return _action->getType();
}

void RegularMergeActionNode::foreachMergeAction(const std::function<void(const scene::merge::IMergeAction::Ptr&)>& functor)
void RegularMergeActionNode::foreachMergeAction(const std::function<void(const merge::IMergeAction::Ptr&)>& functor)
{
if (_action)
{
Expand All @@ -292,18 +292,18 @@ bool RegularMergeActionNode::hasActiveActions()
return _action && _action->isActive();
}

std::shared_ptr<scene::merge::AddCloneToParentAction> RegularMergeActionNode::getAddNodeAction()
std::shared_ptr<merge::AddCloneToParentAction> RegularMergeActionNode::getAddNodeAction()
{
// In case this is a conflicting source action modified an entity that is no longer present, add the old node
auto conflictAction = std::dynamic_pointer_cast<scene::merge::IConflictResolutionAction>(_action);
auto conflictAction = std::dynamic_pointer_cast<merge::IConflictResolutionAction>(_action);

if (conflictAction && conflictAction->getConflictType() == scene::merge::ConflictType::ModificationOfRemovedEntity)
if (conflictAction && conflictAction->getConflictType() == merge::ConflictType::ModificationOfRemovedEntity)
{
return std::dynamic_pointer_cast<scene::merge::AddCloneToParentAction>(conflictAction->getSourceAction());
return std::dynamic_pointer_cast<merge::AddCloneToParentAction>(conflictAction->getSourceAction());
}

// Check the regular action for type AddEntityNode
return std::dynamic_pointer_cast<scene::merge::AddCloneToParentAction>(_action);
return std::dynamic_pointer_cast<merge::AddCloneToParentAction>(_action);
}

void RegularMergeActionNode::addPreviewNodeForAddAction()
Expand All @@ -313,7 +313,7 @@ void RegularMergeActionNode::addPreviewNodeForAddAction()
if (addNodeAction)
{
// Get the clone and add it to the target scene, it needs to be renderable here
scene::addNodeToContainer(_affectedNode, addNodeAction->getParent());
addNodeToContainer(_affectedNode, addNodeAction->getParent());
}
}

Expand All @@ -323,7 +323,7 @@ void RegularMergeActionNode::removePreviewNodeForAddAction()

if (addNodeAction)
{
scene::removeNodeFromParent(_affectedNode);
removeNodeFromParent(_affectedNode);
}
}

Expand Down

0 comments on commit c34710e

Please sign in to comment.