Improve Game Collections

rpcs3/rpcs3qt/game_list_actions.cpp

@@ -608,6 +608,13 @@ void game_list_actions::ShowDiskUsageDialog()
 	});
 }
 
+// How a game collection is listed in either menu: its name, and how many of its games the game list is
+// showing. One pass: a collection may be named "%1", and a chained arg() would substitute into it.
+static QString collection_entry_text(const QString& name, qsizetype count)
+{
+	return QString("%0 (%1)").arg(gui::utils::escape_mnemonics(name), QString::number(count));
+}
+
 void game_list_actions::UpdateGameCollectionMenu(QMenu* menu, QActionGroup* act_group, QMenu* rename_menu, QMenu* remove_menu)
 {
 	const QStringList collections = m_gui_settings->GetGameCollections();
@@ -664,20 +671,25 @@ void game_list_actions::UpdateGameCollectionMenu(QMenu* menu, QActionGroup* act_
 
 	for (const QString& name : collections)
 	{
-		// One pass: a collection may be named "%1", and a chained arg() would substitute into it.
-		add_entry(QString("%0 (%1)").arg(gui::utils::escape_mnemonics(name),
-			QString::number(counts.value(name))), name);
+		add_entry(collection_entry_text(name, counts.value(name)), name);
 	}
 }
 
-void game_list_actions::CreateGameCollection()
+void game_list_actions::CreateGameCollection(const QSet<QString>& serials)
 {
 	const QString name = AskForCollectionName(tr("Create Game Collection"), {},
 		[this](const QString& to) { return m_gui_settings->AddGameCollection(to); });
 
-	if (!name.isEmpty())
+	if (name.isEmpty())
+	{
+		return;
+	}
+
+	game_list_log.notice("Created game collection '%s'", name);
+
+	if (!serials.isEmpty())
 	{
-		game_list_log.notice("Created game collection '%s'", name);
+		ChangeCollectionMembership(serials, name, true);
 	}
 }
 
@@ -732,7 +744,7 @@ void game_list_actions::SelectGameCollection(const QString& name)
 	m_game_list_frame->SetGameCollection(name);
 }
 
-void game_list_actions::AddMoveToCollectionMenu(QMenu* parent, const std::vector<game_info>& games)
+void game_list_actions::AddCollectionMenu(QMenu* parent, const std::vector<game_info>& games)
 {
 	QSet<QString> serials;
 
@@ -750,51 +762,47 @@ void game_list_actions::AddMoveToCollectionMenu(QMenu* parent, const std::vector
 	}
 
 	const QStringList collections = m_gui_settings->GetGameCollections();
+	const QHash<QString, qsizetype> counts = m_game_list_frame->CountGamesPerCollection(collections);
 
-	QMenu* collection_menu = parent->addMenu(tr("&Move To Collection"));
-	collection_menu->setEnabled(!collections.isEmpty());
+	QMenu* collection_menu = parent->addMenu(tr("&Add to Collection"));
 
-	if (collections.isEmpty())
+	// Always there: with no collection yet this is the only thing the submenu can offer
+	connect(collection_menu->addAction(tr("&Create and Add")), &QAction::triggered, this, [this, serials]()
 	{
-		return;
-	}
-
-	// A single game shows the collection it currently sits in. A multi selection is a bulk move to whatever
-	// entry is picked, no matter which collections the games come from, so it shows no state at all.
-	const bool is_single_game = serials.size() == 1;
-	const QString current = is_single_game ? m_gui_settings->GetCollectionOfGame(*serials.cbegin()) : QString();
+		CreateGameCollection(serials);
+	});
 
-	QActionGroup* collection_act_group = is_single_game ? new QActionGroup(collection_menu) : nullptr;
+	if (!collections.isEmpty())
+	{
+		collection_menu->addSeparator();
+	}
 
-	const auto add_entry = [&](const QString& text, const QString& name)
+	for (const QString& collection : collections)
 	{
-		QAction* act = collection_menu->addAction(text);
+		// A multi selection is ticked only once every game in it belongs to the collection, so that the
+		// entry finishes adding the ones that are missing before it starts taking any out
+		const bool is_member = m_gui_settings->GetGamesInCollection(collection).contains(serials);
 
-		if (collection_act_group)
-		{
-			act->setCheckable(true);
-			act->setChecked(name == current);
-			collection_act_group->addAction(act);
-		}
+		QAction* act = collection_menu->addAction(collection_entry_text(collection, counts.value(collection)));
+		act->setCheckable(true);
+		act->setChecked(is_member);
 
-		// Moving one game is trivially undone. Moving a block is not: the games can come from several
-		// collections at once and where each of them was is not recorded anywhere, so that one asks first.
-		connect(act, &QAction::triggered, this, [this, serials, name, is_single_game]()
+		connect(act, &QAction::triggered, this, [this, serials, collection, is_member]()
 		{
-			MoveGamesToCollection(serials, name, !is_single_game);
+			ChangeCollectionMembership(serials, collection, !is_member);
 		});
-	};
-
-	const QHash<QString, qsizetype> counts = m_game_list_frame->CountGamesPerCollection(collections);
+	}
 
-	add_entry(gui_settings::GetAllGamesCollectionLabel(), {});
+	const QString current = m_gui_settings->GetCurrentGameCollection();
 
-	for (const QString& collection : collections)
+	// Acts on the collection the game list is filtered by, which is the one the user is looking at
+	if (!current.isEmpty() && m_gui_settings->GetGamesInCollection(current).intersects(serials))
 	{
-		// One pass: a collection may be named "%1", and a chained arg() would substitute into it.
-		const QString text = QString("%0 (%1)").arg(gui::utils::escape_mnemonics(collection),
-			QString::number(counts.value(collection)));
-		add_entry(text, collection);
+		connect(parent->addAction(tr("&Remove from Collection '%0'").arg(gui::utils::escape_mnemonics(current))),
+			&QAction::triggered, this, [this, serials, current]()
+		{
+			ChangeCollectionMembership(serials, current, false);
+		});
 	}
 }
 
@@ -851,39 +859,40 @@ QString game_list_actions::AskForCollectionName(const QString& title, const QStr
 	return {};
 }
 
-void game_list_actions::MoveGamesToCollection(const QSet<QString>& serials, const QString& name, bool is_interactive)
+void game_list_actions::ChangeCollectionMembership(const QSet<QString>& serials, const QString& name, bool add)
 {
-	const bool moves_anything = !m_gui_settings->GetGamesInCollection(name).contains(serials);
-
-	if (is_interactive && moves_anything)
+	// Adding is undone by the very entry that did it, but a removal takes memberships that were given one
+	// game at a time and cannot be handed back the same way, so a block of them asks first
+	if (!add && serials.size() > 1)
 	{
-		const QString question = name.isEmpty()
-			? tr("Remove %Ln game(s) from their game collection?", "", static_cast<int>(serials.size()))
-			: tr("Move %Ln game(s) to the '%0' game collection?", "", static_cast<int>(serials.size())).arg(name);
+		const QString question = tr("Remove %Ln game(s) from the '%0' game collection?", "",
+			static_cast<int>(serials.size())).arg(name);
 
-		if (gui::utils::plain_message(m_game_list_frame, QMessageBox::Question, tr("Confirm Move"), question,
+		if (gui::utils::plain_message(m_game_list_frame, QMessageBox::Question, tr("Confirm Removal"), question,
 			QMessageBox::Yes | QMessageBox::No) != QMessageBox::Yes)
 		{
 			return;
 		}
 	}
 
-	// The user picked the collection the games already sit in
-	if (!m_gui_settings->MoveGamesToCollection(serials, name))
+	const bool changed = m_gui_settings->SetGameCollectionMembership(serials, name, add);
+
+	// The menu was built before the collection changed under it
+	if (!changed)
 	{
 		return;
 	}
 
-	if (name.isEmpty())
+	if (add)
 	{
-		game_list_log.notice("Removed %d game(s) from their game collection", serials.size());
+		game_list_log.notice("Added %d game(s) to game collection '%s'", serials.size(), name);
 	}
 	else
 	{
-		game_list_log.notice("Moved %d game(s) to game collection '%s'", serials.size(), name);
+		game_list_log.notice("Removed %d game(s) from game collection '%s'", serials.size(), name);
 	}
 
-	// The moved games may have entered or left the collection the game list is filtered by
+	// The games may have entered or left the collection the game list is filtered by
 	m_game_list_frame->ReloadGameCollection();
 }
 

rpcs3/rpcs3qt/game_list_actions.h

@@ -66,8 +66,9 @@ class game_list_actions : QObject
 	// submenus belong to the Manage menu, and are null for the View one, which carries neither.
 	void UpdateGameCollectionMenu(QMenu* menu, QActionGroup* act_group, QMenu* rename_menu, QMenu* remove_menu);
 
-	// Asks for a name until it is accepted or the dialog is dismissed
-	void CreateGameCollection();
+	// Asks for a name until it is accepted or the dialog is dismissed, then adds the given games to what
+	// was created
+	void CreateGameCollection(const QSet<QString>& serials = {});
 
 	// Asks for a new name for a game collection until it is accepted or the dialog is dismissed
 	void RenameGameCollection(const QString& name);
@@ -78,8 +79,10 @@ class game_list_actions : QObject
 	// Makes a game collection the one the game list is filtered by. An empty name shows every game.
 	void SelectGameCollection(const QString& name);
 
-	// Appends the "Move To Collection" submenu, which moves the given games to a user defined game collection
-	void AddMoveToCollectionMenu(QMenu* parent, const std::vector<game_info>& games);
+	// Appends the "Add to Collection" submenu, which puts the given games in a user defined game collection
+	// or takes them out of it, and the entry that removes them from the collection the game list is
+	// filtered by
+	void AddCollectionMenu(QMenu* parent, const std::vector<game_info>& games);
 
 	// NOTES:
 	//   - SetContentList() MUST always be called to set the content's info to be removed by:
@@ -140,8 +143,8 @@ class game_list_actions : QObject
 	QString AskForCollectionName(const QString& title, const QString& initial,
 		const std::function<bool(const QString&)>& accept);
 
-	// Performs what an entry of the "Move To Collection" submenu does. Confirms first when interactive.
-	void MoveGamesToCollection(const QSet<QString>& serials, const QString& name, bool is_interactive);
+	// Puts the given games in a game collection or takes them out of it, then refreshes the game list
+	void ChangeCollectionMembership(const QSet<QString>& serials, const QString& name, bool add);
 
 	void BatchActionBySerials(progress_dialog* pdlg, const std::set<std::string>& serials,
 		QString progressLabel, std::function<bool(const std::string&)> action,

rpcs3/rpcs3qt/game_list_context_menu.cpp

@@ -326,8 +326,8 @@ void game_list_context_menu::show_single_selection_context_menu(const game_info&
 
 	manage_game_menu->addSeparator();
 
-	// Move the game to a user defined game collection
-	m_game_list_actions->AddMoveToCollectionMenu(manage_game_menu, {gameinfo});
+	// Add the game to user defined game collections
+	m_game_list_actions->AddCollectionMenu(manage_game_menu, {gameinfo});
 
 	// Hide/rename game in game list
 	QAction* hide_hidden_serial = manage_game_menu->addAction(tr("&Hide Game In Game List"));
@@ -965,8 +965,8 @@ void game_list_context_menu::show_multi_selection_context_menu(const std::vector
 
 	manage_game_menu->addSeparator();
 
-	// Move the games to a user defined game collection
-	m_game_list_actions->AddMoveToCollectionMenu(manage_game_menu, games);
+	// Add the games to user defined game collections
+	m_game_list_actions->AddCollectionMenu(manage_game_menu, games);
 
 	// Hide game in game list
 	QAction* hide_hidden_serial = manage_game_menu->addAction(tr("&Hide Game In Game List"));

rpcs3/rpcs3qt/gui_settings.cpp

@@ -306,19 +306,6 @@ QSet<QString> gui_settings::GetGamesInCollection(const QString& name) const
 	return gui::utils::list_to_set(GetValue(gui::game_collection, gc_games_prefix + name, QStringList()).toStringList());
 }
 
-QString gui_settings::GetCollectionOfGame(const QString& serial) const
-{
-	for (const QString& collection : GetGameCollections())
-	{
-		if (GetGamesInCollection(collection).contains(serial))
-		{
-			return collection;
-		}
-	}
-
-	return {};
-}
-
 bool gui_settings::CleanupCollections(const QSet<QString>& serials) const
 {
 	const QStringList& collections = GetGameCollections();
@@ -352,42 +339,34 @@ bool gui_settings::CleanupCollections(const QSet<QString>& serials) const
 	return changed;
 }
 
-bool gui_settings::MoveGamesToCollection(const QSet<QString>& serials, const QString& name) const
+bool gui_settings::SetGameCollectionMembership(const QSet<QString>& serials, const QString& name, bool add) const
 {
-	const QStringList collections = GetGameCollections();
-
-	if (serials.isEmpty() || (!name.isEmpty() && !collections.contains(name)))
+	if (serials.isEmpty() || !GetGameCollections().contains(name))
 	{
 		return false;
 	}
 
-	bool changed = false;
+	QSet<QString> games = GetGamesInCollection(name);
+	const qsizetype old_size = games.size();
 
-	for (const QString& collection : collections)
+	if (add)
+	{
+		games.unite(serials);
+	}
+	else
 	{
-		const QSet<QString> old_games = GetGamesInCollection(collection);
-		QSet<QString> games = old_games;
-
 		games.subtract(serials);
-
-		if (collection == name)
-		{
-			games.unite(serials);
-		}
-
-		if (games != old_games)
-		{
-			SetGamesInCollection(collection, games);
-			changed = true;
-		}
 	}
 
-	if (changed)
+	// unite only adds and subtract only removes, so a size that did not move is a set that did not either
+	if (games.size() == old_size)
 	{
-		sync();
+		return false;
 	}
 
-	return changed;
+	SetGamesInCollection(name, games);
+	sync();
+	return true;
 }
 
 QString gui_settings::GetAllGamesCollectionLabel()

rpcs3/rpcs3qt/gui_settings.h

@@ -378,16 +378,13 @@ class gui_settings : public settings
 	void SetCurrentGameCollection(const QString& name, bool sync = true) const;
 	QSet<QString> GetGamesInCollection(const QString& name) const;
 
-	/** Returns the game collection the given game belongs to, empty if it belongs to none */
-	QString GetCollectionOfGame(const QString& serial) const;
-
 	/** Cleans up all game collections according to the provided serials.
 	    Returns whether anything actually changed. */
 	bool CleanupCollections(const QSet<QString>& serials) const;
 
-	/** Moves the given games to a game collection, removing them from any other one. An empty name only
-	    removes them. Returns whether anything actually changed. */
-	bool MoveGamesToCollection(const QSet<QString>& serials, const QString& name) const;
+	/** Puts the given games in a game collection or takes them out of it, leaving every collection they
+	    already belong to alone. Returns whether anything actually changed. */
+	bool SetGameCollectionMembership(const QSet<QString>& serials, const QString& name, bool add) const;
 
 	/** Label of the default game collection entry. Reserved, so it can never name a real collection. */
 	static QString GetAllGamesCollectionLabel();