NOISSUE Comment and bugfix the Resource system

1 files changed, 14 insertions, 28 deletions

diff --git a/logic/resources/ResourceProxyModel.cpp b/logic/resources/ResourceProxyModel.cpp
index 6ff11367..f026d9a9 100644
--- a/logic/resources/ResourceProxyModel.cpp
+++ b/logic/resources/ResourceProxyModel.cpp
@@ -5,8 +5,6 @@
 #include "Resource.h"
 #include "ResourceObserver.h"
 
-//Q_DECLARE_METATYPE(QVector<int>)
-
 class ModelResourceObserver : public ResourceObserver
 {
 public:
@@ -20,6 +18,7 @@ public:
 	{
 		if (m_index.isValid())
 		{
+			// the resource changed, pretend to be the model and notify the views of the update. they will re-poll the model which will return the new resource value
 			QMetaObject::invokeMethod(const_cast<QAbstractItemModel *>(m_index.model()),
 									  "dataChanged", Qt::QueuedConnection,
 									  Q_ARG(QModelIndex, m_index), Q_ARG(QModelIndex, m_index), Q_ARG(QVector<int>, QVector<int>() << m_role));
@@ -39,24 +38,29 @@ ResourceProxyModel::ResourceProxyModel(const int resultTypeId, QObject *parent)
 QVariant ResourceProxyModel::data(const QModelIndex &proxyIndex, int role) const
 {
 	const QModelIndex mapped = mapToSource(proxyIndex);
+	// valid cell that's a Qt::DecorationRole and that contains a non-empty string
 	if (mapped.isValid() && role == Qt::DecorationRole && !mapToSource(proxyIndex).data(role).toString().isEmpty())
 	{
+		// do we already have a resource for this index?
 		if (!m_resources.contains(mapped))
 		{
-			Resource::Ptr res = Resource::create(mapToSource(proxyIndex).data(role).toString())
-					->applyTo(new ModelResourceObserver(proxyIndex, role));
-
-			const QVariant placeholder = mapped.data(PlaceholderRole);
-			if (!placeholder.isNull() && placeholder.type() == QVariant::String)
+			Resource::Ptr placeholder;
+			const QVariant placeholderIdentifier = mapped.data(PlaceholderRole);
+			if (!placeholderIdentifier.isNull() && placeholderIdentifier.type() == QVariant::String)
 			{
-				res->placeholder(Resource::create(placeholder.toString()));
+				placeholder = Resource::create(placeholderIdentifier.toString());
 			}
 
+			// create the Resource and apply the observer for models
+			Resource::Ptr res = Resource::create(mapToSource(proxyIndex).data(role).toString(), placeholder)
+					->applyTo(new ModelResourceObserver(proxyIndex, role));
+
 			m_resources.insert(mapped, res);
 		}
 
 		return m_resources.value(mapped)->getResourceInternal(m_resultTypeId);
 	}
+	// otherwise fall back to the source model
 	return mapped.data(role);
 }
 
@@ -70,7 +74,8 @@ void ResourceProxyModel::setSourceModel(QAbstractItemModel *model)
 	{
 		connect(model, &QAbstractItemModel::dataChanged, this, [this](const QModelIndex &tl, const QModelIndex &br, const QVector<int> &roles)
 		{
-			if (roles.contains(Qt::DecorationRole) || roles.isEmpty())
+			// invalidate resources so that they will be re-created
+			if (roles.contains(Qt::DecorationRole) || roles.contains(PlaceholderRole) || roles.isEmpty())
 			{
 				const QItemSelectionRange range(tl, br);
 				for (const QModelIndex &index : range.indexes())
@@ -78,25 +83,6 @@ void ResourceProxyModel::setSourceModel(QAbstractItemModel *model)
 					m_resources.remove(index);
 				}
 			}
-			else if (roles.contains(PlaceholderRole))
-			{
-				const QItemSelectionRange range(tl, br);
-				for (const QModelIndex &index : range.indexes())
-				{
-					if (m_resources.contains(index))
-					{
-						const QVariant placeholder = index.data(PlaceholderRole);
-						if (!placeholder.isNull() && placeholder.type() == QVariant::String)
-						{
-							m_resources.value(index)->placeholder(Resource::create(placeholder.toString()));
-						}
-						else
-						{
-							m_resources.value(index)->placeholder(nullptr);
-						}
-					}
-				}
-			}
 		});
 	}
 	QIdentityProxyModel::setSourceModel(model);