Skip to content

Commit

Permalink
#5613: Remove _selectedEntity member from EntityInspector, which supp…
Browse files Browse the repository at this point in the history 
…osedly introduces more breakage
  • Loading branch information
@codereader
codereader committed Oct 16, 2021
1 parent 09905eb commit 2f1d985
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 59 deletions.
5 changes: 5 additions & 0 deletions libs/selection/CollectiveSpawnargs.h
View file
Expand Up @@ -72,6 +72,11 @@ class CollectiveSpawnargs
return _sigKeyRemoved;
}

bool containsKey(const std::string& key) const
{
return _entitiesByKey.count(key) > 0;
}

void foreachKey(const std::function<void(const std::string&, const KeyValueSet&)>& functor)
{
for (const auto& pair : _entitiesByKey)
Expand Down
55 changes: 55 additions & 0 deletions libs/selection/EntitySelection.h
View file
Expand Up @@ -2,6 +2,7 @@

#include <list>
#include "inode.h"
#include "ieclass.h"
#include "ientity.h"
#include "iselection.h"
#include "iselectable.h"
Expand Down Expand Up @@ -111,6 +112,11 @@ class EntitySelection final
_trackedEntities.clear();
}

bool empty() const
{
return _trackedEntities.empty();
}

std::size_t size() const
{
return _trackedEntities.size();
Expand All @@ -126,6 +132,55 @@ class EntitySelection final
return _trackedEntities.front().getNode();
}

// Returns non-empty reference if all selected entities share the same eclass
IEntityClassPtr getSingleSharedEntityClass()
{
try
{
IEntityClassPtr result;

foreachEntity([&](Entity* entity)
{
auto eclass = entity->getEntityClass();

if (!result)
{
result = std::move(eclass);
return;
}

if (result != eclass)
{
throw std::runtime_error("Non-unique eclass");
}
});

return result;
}
catch (const std::runtime_error&)
{
return {};
}
}

void foreachEntity(const std::function<void(Entity*)>& functor)
{
for (auto& tracked : _trackedEntities)
{
auto node = tracked.getNode();

if (!node) continue;

auto entity = Node_getEntity(node);
assert(entity != nullptr);

if (entity)
{
functor(entity);
}
}
}

// Rescan the selection system for entities to observe
void update()
{
Expand Down
104 changes: 49 additions & 55 deletions radiant/ui/einspector/EntityInspector.cpp
View file
Expand Up @@ -70,7 +70,8 @@ EntityInspector::EntityInspector() :
_newValueColumn(nullptr),
_keyEntry(nullptr),
_valEntry(nullptr),
_setButton(nullptr)
_setButton(nullptr),
_selectionNeedsUpdate(true)
{}

void EntityInspector::construct()
Expand Down Expand Up @@ -462,7 +463,6 @@ void EntityInspector::onMainFrameShuttingDown()
{
// Clear the selection and unsubscribe from the selection system
changeSelectedEntity(scene::INodePtr(), scene::INodePtr());
_selectedEntity.reset();

GlobalSelectionSystem().removeObserver(this);
_keyValueAddedHandler.disconnect();
Expand Down Expand Up @@ -737,9 +737,6 @@ bool EntityInspector::canUpdateEntity()
// Redraw the GUI elements
void EntityInspector::updateGUIElements()
{
// Update from selection system
getEntityFromSelectionSystem();

auto entityCanBeUpdated = canUpdateEntity();

auto isMergeMode = GlobalMapModule().getEditMode() == IMap::EditMode::Merge;
Expand All @@ -749,7 +746,7 @@ void EntityInspector::updateGUIElements()
// Set the value column back to the default AUTO setting
_valueColumn->SetWidth(wxCOL_WIDTH_AUTOSIZE);

if (!_selectedEntity.expired())
if (!_entitySelection->empty())
{
_editorFrame->Enable(entityCanBeUpdated);
_keyValueTreeView->Enable(true);
Expand All @@ -767,10 +764,12 @@ void EntityInspector::updateGUIElements()
// Update the target entity on any active property editor (#5092)
else if (_currentPropertyEditor)
{
#if 0
auto newEntity = Node_getEntity(_selectedEntity.lock());
assert(newEntity != nullptr);

_currentPropertyEditor->setEntity(newEntity);
#endif
}
}
else // no selected entity
Expand All @@ -790,12 +789,21 @@ void EntityInspector::updateGUIElements()

void EntityInspector::onIdle()
{
if (_selectionNeedsUpdate)
{
_selectionNeedsUpdate = false;
_entitySelection->update();
}

updateGUIElements();
}

// Selection changed callback
void EntityInspector::selectionChanged(const scene::INodePtr& node, bool isComponent)
{
if (isComponent) return; // ignore component changes

_selectionNeedsUpdate = true;
requestIdleCallback();
}

Expand Down Expand Up @@ -853,22 +861,22 @@ void EntityInspector::loadPropertyMap()

void EntityInspector::_onAddKey()
{
assert(!_selectedEntity.expired());
assert(_entitySelection->size() == 1);

Entity* selectedEntity = Node_getEntity(_selectedEntity.lock());
auto selectedEntity = Node_getEntity(_entitySelection->getSingleSelectedEntity());

// Obtain the entity class to provide to the AddPropertyDialog
IEntityClassConstPtr ec = selectedEntity->getEntityClass();
auto ec = selectedEntity->getEntityClass();

// Choose a property, and add to entity with a default value
AddPropertyDialog::PropertyList properties = AddPropertyDialog::chooseProperty(selectedEntity);
auto properties = AddPropertyDialog::chooseProperty(selectedEntity);

for (std::size_t i = 0; i < properties.size(); ++i)
{
const std::string& key = properties[i];

// Add all keys, skipping existing ones to not overwrite any values on the entity
if (selectedEntity->getKeyValue(key) == "" || selectedEntity->isInherited(key))
if (selectedEntity->getKeyValue(key).empty() || selectedEntity->isInherited(key))
{
// Add the keyvalue on the entity (triggering the refresh)
selectedEntity->setKeyValue(key, "-");
Expand All @@ -878,7 +886,7 @@ void EntityInspector::_onAddKey()

bool EntityInspector::_testAddKey()
{
return !_selectedEntity.expired() && canUpdateEntity();
return _entitySelection->size() == 1 && canUpdateEntity();
}

void EntityInspector::_onDeleteKey()
Expand All @@ -888,8 +896,7 @@ void EntityInspector::_onDeleteKey()

if (selectedItems.Count() == 0) return;

assert(!_selectedEntity.expired());
Entity* selectedEntity = Node_getEntity(_selectedEntity.lock());
assert(!_entitySelection->empty());

std::unique_ptr<UndoableCommand> cmd;

Expand All @@ -910,8 +917,7 @@ void EntityInspector::_onDeleteKey()
auto iconAndName = static_cast<wxDataViewIconText>(row[_columns.name]);
auto key = iconAndName.GetText().ToStdString();

// Clear the key after copying
selectedEntity->setKeyValue(key, "");
applyKeyValueToSelection(key, "");
}
}

Expand Down Expand Up @@ -954,8 +960,7 @@ void EntityInspector::_onCutKey()

if (selectedItems.Count() == 0) return;

assert(!_selectedEntity.expired());
Entity* selectedEntity = Node_getEntity(_selectedEntity.lock());
assert(!_entitySelection->empty());

_clipboard.clear();
std::unique_ptr<UndoableCommand> cmd;
Expand All @@ -981,7 +986,7 @@ void EntityInspector::_onCutKey()
_clipboard.emplace_back(key, value);

// Clear the key after copying
selectedEntity->setKeyValue(key, "");
applyKeyValueToSelection(key, "");
}
}

Expand All @@ -1001,7 +1006,7 @@ bool EntityInspector::isItemDeletable(const wxutil::TreeModel::Row& row)

bool EntityInspector::_testNonEmptyAndDeletableSelection()
{
if (_selectedEntity.expired() || !canUpdateEntity()) return false;
if (_entitySelection->empty() || !canUpdateEntity()) return false;

wxDataViewItemArray selectedItems;
_keyValueTreeView->GetSelections(selectedItems);
Expand Down Expand Up @@ -1294,11 +1299,9 @@ void EntityInspector::updateHelpText(const wxutil::TreeModel::Row& row)

std::string key = getSelectedKey();

assert(!_selectedEntity.expired());
auto* selectedEntity = Node_getEntity(_selectedEntity.lock());
auto eclass = _entitySelection->getSingleSharedEntityClass();

auto eclass = selectedEntity->getEntityClass();
assert(eclass);
if (!eclass) return; // non-unique eclass, cannot show any reliable help

if (key == "classname")
{
Expand All @@ -1308,7 +1311,7 @@ void EntityInspector::updateHelpText(const wxutil::TreeModel::Row& row)
else
{
// Find the attribute on the eclass, that's where the descriptions are defined
const EntityClassAttribute& attr = eclass->getAttribute(key);
const auto& attr = eclass->getAttribute(key);

if (!attr.getDescription().empty())
{
Expand All @@ -1326,7 +1329,7 @@ void EntityInspector::_onTreeViewSelectionChanged(wxDataViewEvent& ev)

// Abort if called without a valid entity selection (may happen during
// various cleanup operations).
if (_selectedEntity.expired()) return;
if (_entitySelection->empty()) return;

wxDataViewItemArray selectedItems;
_keyValueTreeView->GetSelections(selectedItems);
Expand Down Expand Up @@ -1364,18 +1367,20 @@ void EntityInspector::_onTreeViewSelectionChanged(wxDataViewEvent& ev)
// Get the type for this key if it exists, and the options
PropertyParms parms = getPropertyParmsForKey(key);

Entity* selectedEntity = Node_getEntity(_selectedEntity.lock());

// If the type was not found, also try looking on the entity class
if (parms.type.empty())
{
IEntityClassConstPtr eclass = selectedEntity->getEntityClass();
parms.type = eclass->getAttribute(key).getType();
auto eclass = _entitySelection->getSingleSharedEntityClass();

if (eclass)
{
parms.type = eclass->getAttribute(key).getType();
}
}

// Construct and add a new PropertyEditor
_currentPropertyEditor = PropertyEditorFactory::create(_editorFrame,
parms.type, selectedEntity, key, parms.options);
parms.type, nullptr, key, parms.options);

if (_currentPropertyEditor)
{
Expand Down Expand Up @@ -1461,31 +1466,19 @@ void EntityInspector::addClassAttribute(const EntityClassAttribute& a)
// Append inherited (entityclass) properties
void EntityInspector::addClassProperties()
{
auto selectedNode = _selectedEntity.lock();

if (!selectedNode)
{
return;
}

Entity* selectedEntity = Node_getEntity(selectedNode);

// Get the entityclass for the current entity
std::string className = selectedEntity->getKeyValue("classname");
IEntityClassPtr eclass = GlobalEntityClassManager().findOrInsert(
className, true
);
// Get the entityclass for the current entities
auto eclass = _entitySelection->getSingleSharedEntityClass();

if (!eclass)
{
rWarning() << "No classname on entity '" << selectedEntity->getKeyValue("name") << "'" << std::endl;
return;
}

// Visit the entity class
eclass->forEachAttribute(
[&](const EntityClassAttribute& a, bool) { addClassAttribute(a); }
);
eclass->forEachAttribute([&] (const EntityClassAttribute& a, bool)
{
addClassAttribute(a);
});
}

// Remove the inherited properties
Expand All @@ -1501,6 +1494,8 @@ void EntityInspector::removeClassProperties()
// Update the selected Entity pointer from the selection system
void EntityInspector::getEntityFromSelectionSystem()
{
return;

_entitySelection->update();

auto numSelectedEntities = _entitySelection->size();
Expand Down Expand Up @@ -1707,17 +1702,16 @@ void EntityInspector::changeSelectedEntity(const scene::INodePtr& newEntity, con

void EntityInspector::handleKeyValueMergeAction(const scene::merge::IEntityKeyValueMergeAction::Ptr& mergeAction)
{
// Remember this action in the map, it will be used in onKeyChange()
_mergeActions[mergeAction->getKey()] = mergeAction;
const auto& key = mergeAction->getKey();

auto selectedEntity = _selectedEntity.lock();
// Remember this action in the map, it will be used in onKeyChange()
_mergeActions[key] = mergeAction;

// Keys added by a merge operation won't be handled in onKeyChange(), so do this here
if (mergeAction->getType() == scene::merge::ActionType::AddKeyValue ||
(mergeAction->getType() == scene::merge::ActionType::ChangeKeyValue && selectedEntity &&
Node_getEntity(selectedEntity)->getKeyValue(mergeAction->getKey()).empty()))
(mergeAction->getType() == scene::merge::ActionType::ChangeKeyValue && !_spawnargs->containsKey(key)))
{
onKeyChange(mergeAction->getKey(), mergeAction->getValue());
onKeyChange(key, mergeAction->getValue());
}
}

Expand Down
7 changes: 3 additions & 4 deletions radiant/ui/einspector/EntityInspector.h
View file
Expand Up @@ -80,10 +80,7 @@ class EntityInspector :
};

private:

// Currently selected entity, this pointer is only non-NULL if the
// current entity selection includes exactly 1 entity.
scene::INodeWeakPtr _selectedEntity;
// Tracking helpers to organise the selected entities and their key values
std::unique_ptr<selection::CollectiveSpawnargs> _spawnargs;
std::unique_ptr<selection::EntitySelection> _entitySelection;

Expand Down Expand Up @@ -166,6 +163,8 @@ class EntityInspector :
std::map<std::string, scene::merge::IEntityKeyValueMergeAction::Ptr> _mergeActions;
std::map<std::string, scene::merge::IConflictResolutionAction::Ptr> _conflictActions;

bool _selectionNeedsUpdate;

private:
bool canUpdateEntity();

Expand Down

0 comments on commit 2f1d985

Please sign in to comment.