Skip to content

Commit

Permalink
#5643: More conflict UI handling, focusing on functionality only.
Browse files Browse the repository at this point in the history
  • Loading branch information
@codereader
codereader committed Jun 20, 2021
1 parent 398f70f commit 24cf022
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 46 deletions.
22 changes: 12 additions & 10 deletions install/ui/mergecontroldialog.fbp
View file
Expand Up @@ -2117,9 +2117,9 @@
</object>
<object class="sizeritem" expanded="1">
<property name="border">5</property>
<property name="flag">wxALL</property>
<property name="flag">wxEXPAND | wxALL</property>
<property name="proportion">0</property>
<object class="wxStaticText" expanded="1">
<object class="wxPanel" expanded="1">
<property name="BottomDockable">1</property>
<property name="LeftDockable">1</property>
<property name="RightDockable">1</property>
Expand All @@ -2143,20 +2143,18 @@
<property name="enabled">1</property>
<property name="fg"></property>
<property name="floatable">1</property>
<property name="font">,90,90,-1,70,0</property>
<property name="font"></property>
<property name="gripper">0</property>
<property name="hidden">0</property>
<property name="id">wxID_ANY</property>
<property name="label">---</property>
<property name="markup">0</property>
<property name="max_size"></property>
<property name="maximize_button">0</property>
<property name="maximum_size"></property>
<property name="min_size"></property>
<property name="minimize_button">0</property>
<property name="minimum_size"></property>
<property name="moveable">1</property>
<property name="name">ConflictDescription</property>
<property name="name">ConflictDescriptionPanel</property>
<property name="pane_border">1</property>
<property name="pane_position"></property>
<property name="pane_size"></property>
Expand All @@ -2165,15 +2163,19 @@
<property name="pos"></property>
<property name="resize">Resizable</property>
<property name="show">1</property>
<property name="size">-1,55</property>
<property name="style"></property>
<property name="size"></property>
<property name="subclass">; ; forward_declare</property>
<property name="toolbar_pane">0</property>
<property name="tooltip"></property>
<property name="window_extra_style"></property>
<property name="window_name"></property>
<property name="window_style"></property>
<property name="wrap">280</property>
<property name="window_style">wxTAB_TRAVERSAL</property>
<object class="wxBoxSizer" expanded="1">
<property name="minimum_size"></property>
<property name="name">ConflictDescriptionSizer</property>
<property name="orient">wxVERTICAL</property>
<property name="permission">none</property>
</object>
</object>
</object>
<object class="sizeritem" expanded="1">
Expand Down
16 changes: 6 additions & 10 deletions install/ui/mergecontroldialog.xrc
View file
Expand Up @@ -434,17 +434,13 @@
</object>
<object class="sizeritem">
<option>0</option>
<flag>wxALL</flag>
<flag>wxEXPAND | wxALL</flag>
<border>5</border>
<object class="wxStaticText" name="ConflictDescription">
<size>-1,55</size>
<font>
<style>normal</style>
<weight>normal</weight>
<underlined>0</underlined>
</font>
<label>---</label>
<wrap>280</wrap>
<object class="wxPanel" name="ConflictDescriptionPanel">
<style>wxTAB_TRAVERSAL</style>
<object class="wxBoxSizer">
<orient>wxVERTICAL</orient>
</object>
</object>
</object>
<object class="sizeritem">
Expand Down
43 changes: 21 additions & 22 deletions radiant/ui/merge/MergeControlDialog.cpp
View file
Expand Up @@ -301,11 +301,9 @@ inline std::string getKeyValue(const scene::merge::IConflictResolutionAction::Pt
return keyConflictAction ? keyConflictAction->getValue() : std::string();
}

inline std::string getConflictDescription(const std::shared_ptr<scene::IMergeActionNode>& node)
inline void addConflictDescription(wxPanel* panel, const std::shared_ptr<scene::IMergeActionNode>& node)
{
std::string text;

if (!node) return text;
if (!node) return;

node->foreachMergeAction([&](const scene::merge::IMergeAction::Ptr& action)
{
Expand All @@ -316,35 +314,33 @@ inline std::string getConflictDescription(const std::shared_ptr<scene::IMergeAct

auto conflictAction = std::dynamic_pointer_cast<scene::merge::IConflictResolutionAction>(action);

auto* text = new wxStaticText(panel, wxID_ANY, "");

switch (conflictAction->getConflictType())
{
case scene::merge::ConflictType::ModificationOfRemovedEntity:
text += text.empty() ? "" : "\n";
text += fmt::format(_("The imported map tries to modify the key values of the entity {0} that has already been deleted here."), getEntityName(conflictAction));
text->SetLabel(fmt::format(_("The imported map tries to modify the key values of the entity {0} that has already been deleted here."), getEntityName(conflictAction)));
break;
case scene::merge::ConflictType::RemovalOfModifiedEntity:
text += text.empty() ? "" : "\n";
text += fmt::format(_("The imported map tries to remove the entity {0} that has been modified in this map."), getEntityName(conflictAction));
text->SetLabel(fmt::format(_("The imported map tries to remove the entity {0} that has been modified in this map."), getEntityName(conflictAction)));
break;
case scene::merge::ConflictType::RemovalOfModifiedKeyValue:
text += text.empty() ? "" : "\n";
text += fmt::format(_("The imported map tries to remove the key {0} on entity {1} but this key has been modified in this map."),
getKeyName(conflictAction), getEntityName(conflictAction));
text->SetLabel(fmt::format(_("The imported map tries to remove the key {0} on entity {1} but this key has been modified in this map."),
getKeyName(conflictAction), getEntityName(conflictAction)));
break;
case scene::merge::ConflictType::ModificationOfRemovedKeyValue:
text += text.empty() ? "" : "\n";
text += fmt::format(_("The imported map tries to modify the key \"{0}\" on entity {1} but this key has already been removed in this map."),
getKeyName(conflictAction), getEntityName(conflictAction));
text->SetLabel(fmt::format(_("The imported map tries to modify the key \"{0}\" on entity {1} but this key has already been removed in this map."),
getKeyName(conflictAction), getEntityName(conflictAction)));
break;
case scene::merge::ConflictType::SettingKeyToDifferentValue:
text += text.empty() ? "" : "\n";
text += fmt::format(_("The imported map tries to set the key \"{0}\" on entity {1} to the value \"{2}\" but this key has already been set to a different value in this map."),
getKeyName(conflictAction), getEntityName(conflictAction), getKeyValue(conflictAction));
text->SetLabel(fmt::format(_("The imported map tries to set the key \"{0}\" on entity {1} to the value \"{2}\" but this key has already been set to a different value in this map."),
getKeyName(conflictAction), getEntityName(conflictAction), getKeyValue(conflictAction)));
break;
}
});

return text;
text->Wrap(panel->GetSize().x);
panel->GetSizer()->Add(text, 0, wxBOTTOM, 6);
});
}

void MergeControlDialog::updateControlSensitivity()
Expand All @@ -361,17 +357,20 @@ void MergeControlDialog::updateControlSensitivity()
findNamedObject<wxWindow>(this, "MergeMapFilename")->Enable(!mergeInProgress);
findNamedObject<wxButton>(this, "RejectSelectionButton")->Enable(mergeInProgress && numSelectedMergeNodes > 0);

auto conflictDescriptionPanel = findNamedObject<wxPanel>(this, "ConflictDescriptionPanel");

auto conflictNode = getSingleSelectedConflictNode();

findNamedObject<wxStaticText>(this, "NoMergeConflictSelected")->Show(conflictNode == nullptr);
findNamedObject<wxStaticText>(this, "ConflictDescription")->Show(conflictNode != nullptr);
findNamedObject<wxButton>(this, "ResolveAcceptButton")->Show(conflictNode != nullptr);
findNamedObject<wxButton>(this, "ResolveRejectButton")->Show(conflictNode != nullptr);
conflictDescriptionPanel->Show(conflictNode != nullptr);

conflictDescriptionPanel->DestroyChildren();

if (conflictNode)
{
findNamedObject<wxStaticText>(this, "ConflictDescription")->SetLabel(getConflictDescription(conflictNode));
findNamedObject<wxStaticText>(this, "ConflictDescription")->Layout();
addConflictDescription(conflictDescriptionPanel, conflictNode);
}

Layout();
Expand Down
29 changes: 26 additions & 3 deletions radiantcore/map/Map.cpp
View file
Expand Up @@ -1087,6 +1087,31 @@ void Map::exportSelected(std::ostream& out, const MapFormatPtr& format)
exporter.exportMap(GlobalSceneGraph().root(), scene::traverseSelected);
}

inline bool actionIsTargetingKeyValue(const scene::merge::IMergeAction::Ptr& action)
{
if (action->getType() == scene::merge::ActionType::AddKeyValue ||
action->getType() == scene::merge::ActionType::RemoveKeyValue ||
action->getType() == scene::merge::ActionType::ChangeKeyValue)
{
return true;
}

// Conflict actions can be targeting key values too
if (action->getType() == scene::merge::ActionType::ConflictResolution)
{
auto conflictAction = std::dynamic_pointer_cast<scene::merge::IConflictResolutionAction>(action);

if (conflictAction->getConflictType() == scene::merge::ConflictType::ModificationOfRemovedKeyValue ||
conflictAction->getConflictType() == scene::merge::ConflictType::RemovalOfModifiedKeyValue ||
conflictAction->getConflictType() == scene::merge::ConflictType::SettingKeyToDifferentValue)
{
return true;
}
}

return false;
}

void Map::createMergeActions()
{
// Group spawnarg actions into one single node if applicable
Expand All @@ -1097,9 +1122,7 @@ void Map::createMergeActions()
{
scene::INodePtr affectedNode = action->getAffectedNode();

if (action->getType() == scene::merge::ActionType::AddKeyValue ||
action->getType() == scene::merge::ActionType::RemoveKeyValue ||
action->getType() == scene::merge::ActionType::ChangeKeyValue)
if (actionIsTargetingKeyValue(action))
{
auto& actions = entityChanges.try_emplace(action->getAffectedNode()).first->second;
actions.push_back(action);
Expand Down
15 changes: 14 additions & 1 deletion radiantcore/map/MergeActionNode.cpp
View file
Expand Up @@ -154,7 +154,20 @@ void KeyValueMergeActionNode::clear()

scene::merge::ActionType KeyValueMergeActionNode::getActionType() const
{
// We report the change key value type since we're doing all kinds of key value changes
// 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 conflict = std::dynamic_pointer_cast<scene::merge::IConflictResolutionAction>(action);

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

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

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

Expand Down

0 comments on commit 24cf022

Please sign in to comment.