Skip to content

Commit

Permalink
Fix #5092: Changing entity selection with Alt-Shift (replace selectio…
Browse files Browse the repository at this point in the history
…n) causes entity property editors to target the old (wrong) entity.

Add setEntity() method to property editors to update their targets while keeping them in memory.
  • Loading branch information
codereader committed Jan 5, 2020
1 parent 6f24b32 commit 6362ff1
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 21 deletions.
8 changes: 8 additions & 0 deletions include/ientityinspector.h
Expand Up @@ -42,6 +42,14 @@ class IPropertyEditor
*/
virtual wxPanel* getWidget() = 0;

/**
* Instructs this editor to focus on a different target, so all operations
* should be performed on the given entity.
* It's not valid to pass nullptrs into this method, doing so will result
* in an exception being thrown.
*/
virtual void setEntity(Entity* entity) = 0;

/**
* Instructs the editor to update its widgets from the edited entity's key values.
*/
Expand Down
15 changes: 11 additions & 4 deletions plugins/dm.editing/AIHeadPropertyEditor.cpp
Expand Up @@ -16,8 +16,8 @@ namespace ui
{

AIHeadPropertyEditor::AIHeadPropertyEditor() :
_widget(NULL),
_entity(NULL)
_widget(nullptr),
_entity(nullptr)
{}

AIHeadPropertyEditor::AIHeadPropertyEditor(wxWindow* parent, Entity* entity, const std::string& key, const std::string& options) :
Expand All @@ -30,14 +30,14 @@ AIHeadPropertyEditor::AIHeadPropertyEditor(wxWindow* parent, Entity* entity, con
// Create the browse button
wxButton* browseButton = new wxButton(_widget, wxID_ANY, _("Choose AI head..."));
browseButton->SetBitmap(wxArtProvider::GetBitmap(GlobalUIManager().ArtIdPrefix() + "icon_model.png"));
browseButton->Connect(wxEVT_BUTTON, wxCommandEventHandler(AIHeadPropertyEditor::onChooseButton), NULL, this);
browseButton->Bind(wxEVT_BUTTON, &AIHeadPropertyEditor::onChooseButton, this);

_widget->GetSizer()->Add(browseButton, 0, wxALIGN_CENTER_VERTICAL);
}

AIHeadPropertyEditor::~AIHeadPropertyEditor()
{
if (_widget != NULL)
if (_widget != nullptr)
{
_widget->Destroy();
}
Expand All @@ -53,6 +53,13 @@ void AIHeadPropertyEditor::updateFromEntity()
// nothing to do
}

void AIHeadPropertyEditor::setEntity(Entity* entity)
{
if (entity == nullptr) throw std::logic_error("No nullptrs allowed as entity argument");

_entity = entity;
}

IPropertyEditorPtr AIHeadPropertyEditor::createNew(wxWindow* parent, Entity* entity,
const std::string& key, const std::string& options)
{
Expand Down
1 change: 1 addition & 0 deletions plugins/dm.editing/AIHeadPropertyEditor.h
Expand Up @@ -30,6 +30,7 @@ class AIHeadPropertyEditor :

wxPanel* getWidget() override;
void updateFromEntity() override;
void setEntity(Entity* entity) override;

AIHeadPropertyEditor(wxWindow* parent, Entity* entity,
const std::string& key, const std::string& options);
Expand Down
15 changes: 11 additions & 4 deletions plugins/dm.editing/AIVocalSetPropertyEditor.cpp
Expand Up @@ -16,8 +16,8 @@ namespace ui
{

AIVocalSetPropertyEditor::AIVocalSetPropertyEditor() :
_widget(NULL),
_entity(NULL)
_widget(nullptr),
_entity(nullptr)
{}

AIVocalSetPropertyEditor::AIVocalSetPropertyEditor(wxWindow* parent, Entity* entity, const std::string& key, const std::string& options) :
Expand All @@ -30,14 +30,14 @@ AIVocalSetPropertyEditor::AIVocalSetPropertyEditor(wxWindow* parent, Entity* ent
// Create the browse button
wxButton* browseButton = new wxButton(_widget, wxID_ANY, _("Select Vocal Set..."));
browseButton->SetBitmap(wxArtProvider::GetBitmap(GlobalUIManager().ArtIdPrefix() + "icon_sound.png"));
browseButton->Connect(wxEVT_BUTTON, wxCommandEventHandler(AIVocalSetPropertyEditor::onChooseButton), NULL, this);
browseButton->Bind(wxEVT_BUTTON, &AIVocalSetPropertyEditor::onChooseButton, this);

_widget->GetSizer()->Add(browseButton, 0, wxALIGN_CENTER_VERTICAL);
}

AIVocalSetPropertyEditor::~AIVocalSetPropertyEditor()
{
if (_widget != NULL)
if (_widget != nullptr)
{
_widget->Destroy();
}
Expand All @@ -53,6 +53,13 @@ void AIVocalSetPropertyEditor::updateFromEntity()
// Nothing to do
}

void AIVocalSetPropertyEditor::setEntity(Entity* entity)
{
if (entity == nullptr) throw std::logic_error("No nullptrs allowed as entity argument");

_entity = entity;
}

IPropertyEditorPtr AIVocalSetPropertyEditor::createNew(wxWindow* parent, Entity* entity,
const std::string& key, const std::string& options)
{
Expand Down
1 change: 1 addition & 0 deletions plugins/dm.editing/AIVocalSetPropertyEditor.h
Expand Up @@ -33,6 +33,7 @@ class AIVocalSetPropertyEditor :

wxPanel* getWidget() override;
void updateFromEntity() override;
void setEntity(Entity* entity) override;

IPropertyEditorPtr createNew(wxWindow* parent, Entity* entity,
const std::string& key,
Expand Down
14 changes: 10 additions & 4 deletions radiant/ui/einspector/EntityInspector.cpp
Expand Up @@ -586,14 +586,20 @@ void EntityInspector::updateGUIElements()
_keyValueTreeView->Enable(true);
_showInheritedCheckbox->Enable(true);
_showHelpColumnCheckbox->Enable(true);

// Update the target entity on any active property editor (#5092)
if (_currentPropertyEditor)
{
auto newEntity = Node_getEntity(_selectedEntity.lock());
assert(newEntity != nullptr);

_currentPropertyEditor->setEntity(newEntity);
}
}
else // no selected entity
{
// Remove the displayed PropertyEditor
if (_currentPropertyEditor)
{
_currentPropertyEditor = PropertyEditorPtr();
}
_currentPropertyEditor.reset();

_helpText->SetValue("");

Expand Down
31 changes: 22 additions & 9 deletions radiant/ui/einspector/PropertyEditor.cpp
Expand Up @@ -10,22 +10,22 @@ namespace ui
{

PropertyEditor::PropertyEditor() :
_mainWidget(NULL),
_entity(NULL)
_mainWidget(nullptr),
_entity(nullptr)
{}

PropertyEditor::PropertyEditor(Entity* entity) :
_mainWidget(NULL),
_mainWidget(nullptr),
_entity(entity)
{}

PropertyEditor::~PropertyEditor()
{
// Destroy the widget
if (_mainWidget != NULL)
if (_mainWidget != nullptr)
{
_mainWidget->Destroy();
_mainWidget = NULL;
_mainWidget = nullptr;
}
}

Expand All @@ -37,7 +37,7 @@ void PropertyEditor::setMainWidget(wxPanel* widget)
// to forget about our reference to avoid double deletions
_mainWidget->Bind(wxEVT_DESTROY, [&] (wxWindowDestroyEvent&)
{
_mainWidget = NULL;
_mainWidget = nullptr;
});
}

Expand All @@ -47,14 +47,27 @@ wxPanel* PropertyEditor::getWidget()
return _mainWidget;
}

void PropertyEditor::setEntity(Entity* entity)
{
if (entity == nullptr) throw std::logic_error("No nullptrs allowed as entity argument");

if (_entity != entity)
{
_entity = entity;

// Let any subclasses update themselves now that the entity got changed
updateFromEntity();
}
}

std::string PropertyEditor::getKeyValue(const std::string& key)
{
return (_entity != NULL) ? _entity->getKeyValue(key) : "";
return _entity != nullptr ? _entity->getKeyValue(key) : std::string();
}

void PropertyEditor::setKeyValue(const std::string& key, const std::string& value)
{
if (_entity == NULL) return;
if (_entity == nullptr) return;

UndoableCommand cmd("setProperty");

Expand All @@ -74,7 +87,7 @@ void PropertyEditor::constructBrowseButtonPanel(wxWindow* parent, const std::str
// Button with image
wxButton* button = new wxButton(mainVBox, wxID_ANY, label);
button->SetBitmap(bitmap);
button->Connect(wxEVT_BUTTON, wxCommandEventHandler(PropertyEditor::_onBrowseButtonClick), NULL, this);
button->Bind(wxEVT_BUTTON, &PropertyEditor::_onBrowseButtonClick, this);

mainVBox->GetSizer()->Add(button, 0, wxALIGN_CENTER_VERTICAL);
}
Expand Down
2 changes: 2 additions & 0 deletions radiant/ui/einspector/PropertyEditor.h
Expand Up @@ -91,6 +91,8 @@ class PropertyEditor :
{
// nothing by default, override in subclasses if needed
}

virtual void setEntity(Entity* entity) override;
};

} // namespace

0 comments on commit 6362ff1

Please sign in to comment.