Skip to content

Commit

Permalink
#5827: ItemValueProxy uses a setAttr() method to set style attributes
Browse files Browse the repository at this point in the history
Although overloading allows it, "assigning" twice to the same object
to perform two different operations yields very confusing code, and
violates the expectation that an overloaded operator should do something
similar to what the operator would do for a primitive type.
  • Loading branch information
Matthew Mott committed Oct 19, 2022
1 parent 032dc98 commit fdb7f85
Show file tree
Hide file tree
Showing 15 changed files with 41 additions and 53 deletions.
2 changes: 1 addition & 1 deletion libs/wxutil/EntityClassChooser.cpp
Expand Up @@ -109,7 +109,7 @@ class EntityClassTreePopulator:

row[_columns.isFolder] = isFolder;
row[_columns.isFavourite] = isFavourite;
row[_columns.iconAndName] = TreeViewItemStyle::Declaration(isFavourite); // assign attributes
row[_columns.iconAndName].setAttr(TreeViewItemStyle::Declaration(isFavourite)); // assign attributes
row.SendItemAdded();
}
);
Expand Down
2 changes: 1 addition & 1 deletion libs/wxutil/dataview/KeyValueTable.cpp
Expand Up @@ -58,7 +58,7 @@ void KeyValueTable::Append(const std::string& key, const std::string& value)
bold.SetBold(true);

row[COLUMNS().key] = key;
row[COLUMNS().key] = bold;
row[COLUMNS().key].setAttr(bold);
row[COLUMNS().value] = value;

row.SendItemAdded();
Expand Down
2 changes: 1 addition & 1 deletion libs/wxutil/dataview/ResourceTreeView.cpp
Expand Up @@ -513,7 +513,7 @@ void ResourceTreeView::SetFavouriteRecursively(TreeModel::Row& row, bool isFavou
void ResourceTreeView::SetFavourite(TreeModel::Row& row, bool isFavourite)
{
row[_columns.isFavourite] = isFavourite;
row[_columns.iconAndName] = TreeViewItemStyle::Declaration(isFavourite);
row[_columns.iconAndName].setAttr(TreeViewItemStyle::Declaration(isFavourite));

// Keep track of this choice
if (isFavourite)
Expand Down
2 changes: 1 addition & 1 deletion libs/wxutil/dataview/ThreadedDeclarationTreePopulator.h
Expand Up @@ -79,7 +79,7 @@ class ThreadedDeclarationTreePopulator :
auto isFavourite = IsFavourite(declName);

row[_columns.iconAndName] = wxVariant(wxDataViewIconText(leafName, !isFolder ? _declIcon : _folderIcon));
row[_columns.iconAndName] = TreeViewItemStyle::Declaration(isFavourite);
row[_columns.iconAndName].setAttr(TreeViewItemStyle::Declaration(isFavourite));
row[_columns.fullName] = fullPath;
row[_columns.leafName] = leafName;
row[_columns.declName] = declName;
Expand Down
2 changes: 1 addition & 1 deletion libs/wxutil/dataview/TreeModel.h
Expand Up @@ -187,7 +187,7 @@ class TreeModel :
}

// get/set operators for dataview attributes
ItemValueProxy& operator=(const wxDataViewItemAttr& attr)
ItemValueProxy& setAttr(const wxDataViewItemAttr& attr)
{
_model.SetAttr(_item, _column.getColumnIndex(), attr);
return *this;
Expand Down
4 changes: 2 additions & 2 deletions plugins/dm.difficulty/DifficultySettings.cpp
Expand Up @@ -166,7 +166,7 @@ void DifficultySettings::updateTreeModel()
colour.SetColour(setting.isDefault ? wxColor(112,112,112) : wxColor(0,0,0));

row[_columns.description] = setting.getDescString() + (overridden ? _(" (overridden)") : "");
row[_columns.description] = colour;
row[_columns.description].setAttr(colour);

row[_columns.classname] = setting.className;
row[_columns.settingId] = setting.id;
Expand Down Expand Up @@ -285,7 +285,7 @@ wxDataViewItem DifficultySettings::insertClassName(const std::string& className,
black.SetColour(wxColor(0,0,0));

row[_columns.description] = className;
row[_columns.description] = black;
row[_columns.description].setAttr(black);

row[_columns.classname] = className;
row[_columns.settingId] = -1;
Expand Down
4 changes: 2 additions & 2 deletions plugins/dm.stimresponse/SREntity.cpp
Expand Up @@ -239,10 +239,10 @@ void SREntity::writeToListRow(wxutil::TreeModel::Row& row, StimResponse& sr)
const SRListColumns& cols = getColumns();

row[cols.index] = sr.getIndex();
row[cols.index] = colour;
row[cols.index].setAttr(colour);
row[cols.srClass] = wxVariant(wxutil::GetLocalBitmap(classIcon));
row[cols.caption] = wxVariant(wxDataViewIconText(stimTypeStr, icon));
row[cols.caption] = colour;
row[cols.caption].setAttr(colour);
row[cols.inherited] = sr.inherited();
}

Expand Down
4 changes: 2 additions & 2 deletions radiant/ui/eclasstree/EClassTree.cpp
Expand Up @@ -138,10 +138,10 @@ void EClassTree::addToListStore(const EntityClassAttribute& attr, bool inherited
colour.SetColour(inherited ? wxColor(127, 127, 127) : wxColor(0, 0, 0));

row[_propertyColumns.name] = attr.getName();
row[_propertyColumns.name] = colour;
row[_propertyColumns.name].setAttr(colour);

row[_propertyColumns.value] = attr.getValue();
row[_propertyColumns.value] = colour;
row[_propertyColumns.value].setAttr(colour);

row[_propertyColumns.inherited] = inherited ? "1" : "0";

Expand Down
2 changes: 1 addition & 1 deletion radiant/ui/einspector/AddPropertyDialog.cpp
Expand Up @@ -161,7 +161,7 @@ void AddPropertyDialog::populateTreeView()
blueBold.SetBold(true);

defRoot[_columns.displayName] = wxVariant(wxDataViewIconText(cName, folderIcon));
defRoot[_columns.displayName] = blueBold;
defRoot[_columns.displayName].setAttr(blueBold);
defRoot[_columns.propertyName] = "";
defRoot[_columns.description] = _(CUSTOM_PROPERTY_TEXT);

Expand Down
42 changes: 16 additions & 26 deletions radiant/ui/einspector/EntityInspector.cpp
Expand Up @@ -222,16 +222,16 @@ void EntityInspector::onKeyChange(const std::string& key, const std::string& val
setOldAndNewValueColumns(row, key, style);

// Apply background style to all other columns
row[_columns.name] = style;
row[_columns.booleanValue] = style;
row[_columns.name].setAttr(style);
row[_columns.booleanValue].setAttr(style);

// Before applying the style to the value, check if the value is ambiguous
if (isMultiValue)
{
wxutil::TreeViewItemStyle::ApplyKeyValueAmbiguousStyle(style);
}

row[_columns.value] = style;
row[_columns.value].setAttr(style);
row[_columns.isInherited] = false;

updateKeyType(row);
Expand Down Expand Up @@ -306,22 +306,22 @@ void EntityInspector::setOldAndNewValueColumns(wxutil::TreeModel::Row& row, cons
if (action->second->getType() == scene::merge::ActionType::AddKeyValue)
{
row[_columns.oldValue] = std::string(); // no old value to show
row[_columns.oldValue] = style;
row[_columns.oldValue].setAttr(style);
}
else
{
wxDataViewItemAttr oldAttr = style;
wxutil::TreeViewItemStyle::SetStrikethrough(oldAttr, true);
row[_columns.oldValue] = action->second->getUnchangedValue();
row[_columns.oldValue] = oldAttr;
row[_columns.oldValue].setAttr(oldAttr);
}

row[_columns.newValue] = action->second->getValue();

wxDataViewItemAttr newAttr = style;
newAttr.SetBold(true);

row[_columns.newValue] = newAttr;
row[_columns.newValue].setAttr(newAttr);
}
else
{
Expand Down Expand Up @@ -1326,13 +1326,9 @@ void EntityInspector::_onEntryActivate(wxCommandEvent& ev)
void EntityInspector::handleShowInheritedChanged()
{
if (_showInheritedCheckbox->IsChecked())
{
addClassProperties();
}
else
{
removeClassProperties();
}
}

void EntityInspector::updateHelpTextPanel()
Expand Down Expand Up @@ -1597,8 +1593,6 @@ void EntityInspector::addClassAttribute(const EntityClassAttribute& a)

auto row = _kvStore->AddItem();

auto style = wxutil::TreeViewItemStyle::Inherited();

row[_columns.name] = wxVariant(wxDataViewIconText(a.getName(), _emptyIcon));
row[_columns.value] = a.getValue();

Expand All @@ -1616,8 +1610,11 @@ void EntityInspector::addClassAttribute(const EntityClassAttribute& a)
row[_columns.booleanValue].setEnabled(false);
}

row[_columns.name] = style;
row[_columns.value] = style;
// Set style attributes
auto style = wxutil::TreeViewItemStyle::Inherited();
row[_columns.name].setAttr(style);
row[_columns.value].setAttr(style);

row[_columns.oldValue] = std::string();
row[_columns.newValue] = std::string();

Expand All @@ -1629,19 +1626,12 @@ void EntityInspector::addClassAttribute(const EntityClassAttribute& a)
// Append inherited (entityclass) properties
void EntityInspector::addClassProperties()
{
// Get the entityclass for the current entities
auto eclass = _entitySelection->getSingleSharedEntityClass();

if (!eclass)
{
return;
// Visit the entityclass for the current entities
if (auto eclass = _entitySelection->getSingleSharedEntityClass(); eclass) {
eclass->forEachAttribute(
[&](const EntityClassAttribute& a, bool) { addClassAttribute(a); }
);
}

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

// Remove the inherited properties
Expand Down
4 changes: 2 additions & 2 deletions radiant/ui/filters/editor/FilterDialog.cpp
Expand Up @@ -128,8 +128,8 @@ void FilterDialog::update()
row[_columns.name] = i->first;
row[_columns.state] = filter.state ? std::string(_("enabled")) : std::string(_("disabled"));

row[_columns.name] = filter.readOnly ? grey : black;
row[_columns.state] = filter.readOnly ? grey : black;
row[_columns.name].setAttr(filter.readOnly ? grey : black);
row[_columns.state].setAttr(filter.readOnly ? grey : black);

row[_columns.readonly] = filter.readOnly;

Expand Down
8 changes: 5 additions & 3 deletions radiant/ui/layers/LayerControlDialog.cpp
Expand Up @@ -205,7 +205,7 @@ class LayerControlDialog::TreePopulator

if (_activeLayerId == layerId)
{
row[_columns.name] = wxutil::TreeViewItemStyle::ActiveItemStyle();
row[_columns.name].setAttr(wxutil::TreeViewItemStyle::ActiveItemStyle());
}

if (_layerManager.layerIsVisible(layerId))
Expand Down Expand Up @@ -304,8 +304,10 @@ void LayerControlDialog::update()

row[_columns.name] = layerName;

row[_columns.name] = activeLayerId == layerId ?
wxutil::TreeViewItemStyle::ActiveItemStyle() : wxDataViewItemAttr(); // no style
row[_columns.name].setAttr(
activeLayerId == layerId ? wxutil::TreeViewItemStyle::ActiveItemStyle()
: wxDataViewItemAttr() // no style
);

if (layerManager.layerIsVisible(layerId))
{
Expand Down
10 changes: 3 additions & 7 deletions radiant/ui/materials/editor/MaterialEditor.cpp
Expand Up @@ -1206,7 +1206,7 @@ void MaterialEditor::setupMaterialStageView()
wxDataViewItemAttr globalStyle;
globalStyle.SetColour(wxColor(15, 15, 15));
globalStyle.SetBold(true);
row[STAGE_COLS().name] = globalStyle;
row[STAGE_COLS().name].setAttr(globalStyle);

row.SendItemAdded();

Expand Down Expand Up @@ -3042,13 +3042,9 @@ void MaterialEditor::updateMaterialTreeItem()
wxutil::TreeModel::Row row(item, *_treeView->GetModel());

if (!row[columns.isFolder].getBool())
{
row[columns.iconAndName] = wxutil::TreeViewItemStyle::Modified(isModified);
}
row[columns.iconAndName].setAttr(wxutil::TreeViewItemStyle::Modified(isModified));
else
{
row[columns.iconAndName] = wxDataViewItemAttr();
}
row[columns.iconAndName].setAttr(wxDataViewItemAttr());

wxDataViewIconText value = row[columns.iconAndName];

Expand Down
4 changes: 2 additions & 2 deletions radiant/ui/modelselector/ModelDataInserter.h
Expand Up @@ -72,7 +72,7 @@ class ModelDataInserter :

// Pixbuf depends on model type
row[_columns.iconAndName] = wxVariant(wxDataViewIconText(displayName, isExplicit ? _modelIcon : _folderIcon));
row[_columns.iconAndName] = wxutil::TreeViewItemStyle::Declaration(isFavourite); // assign attributes
row[_columns.iconAndName].setAttr(wxutil::TreeViewItemStyle::Declaration(isFavourite)); // assign attributes
row[_columns.fullName] = fullPath;
row[_columns.modelPath] = fullPath;
row[_columns.leafName] = displayName;
Expand All @@ -97,7 +97,7 @@ class ModelDataInserter :
isFavourite = isExplicit && _favourites.count(fullSkinPath) > 0;

skinRow[_columns.iconAndName] = wxVariant(wxDataViewIconText(skinName, _skinIcon));
skinRow[_columns.iconAndName] = wxutil::TreeViewItemStyle::Declaration(isFavourite); // assign attributes
skinRow[_columns.iconAndName].setAttr(wxutil::TreeViewItemStyle::Declaration(isFavourite)); // assign attributes
skinRow[_columns.fullName] = fullSkinPath; // model path + skin
skinRow[_columns.modelPath] = fullPath; // this is the model path
skinRow[_columns.leafName] = skinName;
Expand Down
2 changes: 1 addition & 1 deletion radiant/ui/particles/ParticleEditor.cpp
Expand Up @@ -1041,7 +1041,7 @@ void ParticleEditor::reloadStageList()
colour.SetColour(stage->isVisible() ? wxColour(0, 0, 0) : wxColour(127, 127, 127));

row[STAGE_COLS().name] = fmt::format("Stage {0}", static_cast<int>(i));
row[STAGE_COLS().name] = colour;
row[STAGE_COLS().name].setAttr(colour);

row[STAGE_COLS().index] = static_cast<int>(i);
row[STAGE_COLS().visible] = true;
Expand Down

0 comments on commit fdb7f85

Please sign in to comment.