sceNpTrophyGetTrophyIcon: SCE_NP_TROPHY_ERROR_HIDDEN

And prevent some possible segfaults

Utilities/rXml.cpp

@@ -59,9 +59,9 @@ rXmlDocument::rXmlDocument()
 {
 }
 
-void rXmlDocument::Read(const std::string& data)
+pugi::xml_parse_result rXmlDocument::Read(const std::string& data)
 {
-	handle.load_buffer(data.data(), data.size());
+	return handle.load_buffer(data.data(), data.size());
 }
 
 std::shared_ptr<rXmlNode> rXmlDocument::GetRoot()

Utilities/rXml.h

@@ -34,8 +34,8 @@ struct rXmlDocument
 	rXmlDocument();
 	rXmlDocument(const rXmlDocument& other) = delete;
 	rXmlDocument &operator=(const rXmlDocument& other) = delete;
-	void Read(const std::string& data);
+	pugi::xml_parse_result Read(const std::string& data);
-	std::shared_ptr<rXmlNode> GetRoot();
+	virtual std::shared_ptr<rXmlNode> GetRoot();
 
 	pugi::xml_document handle{};
 };

rpcs3/Emu/Cell/Modules/sceNpTrophy.cpp

@@ -746,21 +746,25 @@ error_code sceNpTrophyGetGameInfo(u32 context, u32 handle, vm::ptr<SceNpTrophyGa
 		return SCE_NP_TROPHY_ERROR_INVALID_ARGUMENT;
 	}
 
-	fs::file config(vfs::get("/dev_hdd0/home/" + Emu.GetUsr() + "/trophy/" + ctxt->trp_name + "/TROPCONF.SFM"));
+	const std::string config_path = vfs::get("/dev_hdd0/home/" + Emu.GetUsr() + "/trophy/" + ctxt->trp_name + "/TROPCONF.SFM");
+
+	fs::file config(config_path);
 
 	if (!config)
 	{
-		return SCE_NP_TROPHY_ERROR_CONF_DOES_NOT_EXIST;
+		return { SCE_NP_TROPHY_ERROR_CONF_DOES_NOT_EXIST, config_path };
 	}
 
-	rXmlDocument doc;
+	trophy_xml_document doc{};
-	doc.Read(config.to_string());
+	pugi::xml_parse_result res = doc.Read(config.to_string());
+	if (!res)
+	{
+		sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM: %s", config_path);
+		// TODO: return some error
+	}
 
 	auto trophy_base = doc.GetRoot();
-	if (trophy_base->GetChildren()->GetName() == "trophyconf")
+	ensure(trophy_base);
-	{
-		trophy_base = trophy_base->GetChildren();
-	}
 
 	if (details)
 		*details = {};
@@ -979,24 +983,28 @@ static error_code NpTrophyGetTrophyInfo(const trophy_context_t* ctxt, s32 trophy
 		return SCE_NP_TROPHY_ERROR_CONTEXT_NOT_REGISTERED;
 	}
 
-	fs::file config(vfs::get("/dev_hdd0/home/" + Emu.GetUsr() + "/trophy/" + ctxt->trp_name + "/TROPCONF.SFM"));
+	const std::string config_path = vfs::get("/dev_hdd0/home/" + Emu.GetUsr() + "/trophy/" + ctxt->trp_name + "/TROPCONF.SFM");
+
+	fs::file config(config_path);
 
 	if (!config)
 	{
-		return SCE_NP_TROPHY_ERROR_CONF_DOES_NOT_EXIST;
+		return { SCE_NP_TROPHY_ERROR_CONF_DOES_NOT_EXIST, config_path };
 	}
 
 	SceNpTrophyDetails tmp_details{};
 	SceNpTrophyData tmp_data{};
 
-	rXmlDocument doc;
+	trophy_xml_document doc{};
-	doc.Read(config.to_string());
+	pugi::xml_parse_result res = doc.Read(config.to_string());
+	if (!res)
+	{
+		sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM: %s", config_path);
+		// TODO: return some error
+	}
 
 	auto trophy_base = doc.GetRoot();
-	if (trophy_base->GetChildren()->GetName() == "trophyconf")
+	ensure(trophy_base);
-	{
-		trophy_base = trophy_base->GetChildren();
-	}
 
 	bool found = false;
 	for (std::shared_ptr<rXmlNode> n = trophy_base->GetChildren(); n; n = n->GetNext())
@@ -1232,8 +1240,36 @@ error_code sceNpTrophyGetTrophyIcon(u32 context, u32 handle, s32 trophyId, vm::p
 
 	if (!ctxt->tropusr->GetTrophyUnlockState(trophyId))
 	{
-		bool hidden = false; // TODO obtain this value
+		const std::string config_path = vfs::get("/dev_hdd0/home/" + Emu.GetUsr() + "/trophy/" + ctxt->trp_name + "/TROPCONF.SFM");
-		return hidden ? SCE_NP_TROPHY_ERROR_HIDDEN : SCE_NP_TROPHY_ERROR_LOCKED;
+
+		fs::file config(config_path);
+		if (config)
+		{
+			trophy_xml_document doc{};
+			pugi::xml_parse_result res = doc.Read(config.to_string());
+			if (!res)
+			{
+				sceNpTrophy.error("sceNpTrophyGetGameInfo: Failed to read TROPCONF.SFM: %s", config_path);
+				// TODO: return some error
+			}
+
+			auto trophy_base = doc.GetRoot();
+			ensure(trophy_base);
+
+			for (std::shared_ptr<rXmlNode> n = trophy_base->GetChildren(); n; n = n->GetNext())
+			{
+				if (n->GetName() == "trophy" && trophyId == atoi(n->GetAttribute("id").c_str()) && n->GetAttribute("hidden")[0] == 'y')
+				{
+					return SCE_NP_TROPHY_ERROR_HIDDEN;
+				}
+			}
+		}
+		else
+		{
+			// TODO: Maybe return SCE_NP_TROPHY_ERROR_CONF_DOES_NOT_EXIST
+		}
+
+		return SCE_NP_TROPHY_ERROR_LOCKED;
 	}
 
 	fs::file icon_file(vfs::get("/dev_hdd0/home/" + Emu.GetUsr() + "/trophy/" + ctxt->trp_name + fmt::format("/TROP%03d.PNG", trophyId)));

rpcs3/Loader/TROPUSR.cpp

@@ -1,5 +1,4 @@
 #include "stdafx.h"
-#include "Utilities/rXml.h"
 #include "Emu/VFS.h"
 #include "TROPUSR.h"
 
@@ -10,6 +9,25 @@ enum : u32
 	TROPUSR_MAGIC = 0x818F54AD
 };
 
+std::shared_ptr<rXmlNode> trophy_xml_document::GetRoot()
+{
+	auto trophy_base = rXmlDocument::GetRoot();
+	ensure(trophy_base);
+
+	if (auto trophy_conf = trophy_base->GetChildren();
+		trophy_conf && trophy_conf->GetName() == "trophyconf")
+	{
+		trophy_base = trophy_conf;
+	}
+	else
+	{
+		trp_log.error("trophy_xml_document: Root name does not match trophyconf in trophy. Name: %s", trophy_conf ? trophy_conf->GetName() : trophy_base->GetName());
+		// TODO: return nullptr or is this possible?
+	}
+
+	return trophy_base;
+}
+
 TROPUSRLoader::load_result TROPUSRLoader::Load(const std::string& filepath, const std::string& configpath)
 {
 	const std::string& path = vfs::get(filepath);
@@ -148,17 +166,19 @@ bool TROPUSRLoader::Generate(const std::string& filepath, const std::string& con
 		return false;
 	}
 
-	rXmlDocument doc;
+	trophy_xml_document doc{};
-	doc.Read(config.to_string());
+	pugi::xml_parse_result res = doc.Read(config.to_string());
+	if (!res)
+	{
+		trp_log.error("TROPUSRLoader::Generate: Failed to read file: %s", filepath);
+		return false;
+	}
 
 	m_table4.clear();
 	m_table6.clear();
 
 	auto trophy_base = doc.GetRoot();
-	if (trophy_base->GetChildren()->GetName() == "trophyconf")
+	ensure(trophy_base);
-	{
-		trophy_base = trophy_base->GetChildren();
-	}
 
 	for (std::shared_ptr<rXmlNode> n = trophy_base->GetChildren(); n; n = n->GetNext())
 	{
@@ -247,14 +267,16 @@ u32 TROPUSRLoader::GetUnlockedPlatinumID(u32 trophy_id, const std::string& confi
 		return invalid_trophy_id;
 	}
 
-	rXmlDocument doc;
+	trophy_xml_document doc{};
-	doc.Read(config.to_string());
+	pugi::xml_parse_result res = doc.Read(config.to_string());
+	if (!res)
+	{
+		trp_log.error("TROPUSRLoader::GetUnlockedPlatinumID: Failed to read file: %s", config_path);
+		return invalid_trophy_id;
+	}
 
 	auto trophy_base = doc.GetRoot();
-	if (trophy_base->GetChildren()->GetName() == "trophyconf")
+	ensure(trophy_base);
-	{
-		trophy_base = trophy_base->GetChildren();
-	}
 
 	const usz trophy_count = m_table4.size();
 

rpcs3/Loader/TROPUSR.h

@@ -1,4 +1,7 @@
 #pragma once
+#include "stdafx.h"
+#include "Utilities/rXml.h"
+#include "Utilities/File.h"
 
 struct TROPUSRHeader
 {
@@ -54,6 +57,12 @@ struct TROPUSREntry6
 	// Note: One of the fields should hold a flag showing whether the trophy is hidden or not
 };
 
+struct trophy_xml_document : public rXmlDocument
+{
+	trophy_xml_document() : rXmlDocument() {}
+	std::shared_ptr<rXmlNode> GetRoot() override;
+};
+
 class TROPUSRLoader
 {
 	enum trophy_grade : u32

rpcs3/rpcs3qt/game_list.cpp

@@ -10,7 +10,7 @@ void game_list::clear_list()
 
 void game_list::mousePressEvent(QMouseEvent *event)
 {
-	if (!indexAt(event->pos()).isValid() || !itemAt(event->pos())->data(Qt::UserRole).isValid())
+	if (QTableWidgetItem* item = itemAt(event->pos()); !item || !item->data(Qt::UserRole).isValid())
 	{
 		clearSelection();
 		setCurrentItem(nullptr); // Needed for currentItemChanged

rpcs3/rpcs3qt/trophy_manager_dialog.cpp

@@ -1,3 +1,4 @@
+#include "stdafx.h"
 #include "trophy_manager_dialog.h"
 #include "custom_table_widget_item.h"
 #include "table_item_delegate.h"
@@ -13,8 +14,6 @@
 #include "Emu/system_utils.hpp"
 #include "Emu/Cell/Modules/sceNpTrophy.h"
 
-#include "Loader/TROPUSR.h"
-
 #include <QtConcurrent>
 #include <QFutureWatcher>
 #include <QHeaderView>
@@ -409,12 +408,16 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name)
 	}
 
 	// Get game name
-	game_trophy_data->trop_config.Read(config.to_string());
+	pugi::xml_parse_result res = game_trophy_data->trop_config.Read(config.to_string());
-	std::shared_ptr<rXmlNode> trophy_base = game_trophy_data->trop_config.GetRoot();
+	if (!res)
-	if (trophy_base->GetChildren()->GetName() == "trophyconf")
 	{
-		trophy_base = trophy_base->GetChildren();
+		gui_log.error("Failed to read trophy xml: %s", tropconf_path);
+		return false;
 	}
+
+	std::shared_ptr<rXmlNode> trophy_base = game_trophy_data->trop_config.GetRoot();
+	ensure(trophy_base);
+
 	for (std::shared_ptr<rXmlNode> n = trophy_base->GetChildren(); n; n = n->GetNext())
 	{
 		if (n->GetName() == "title-name")
@@ -872,15 +875,7 @@ void trophy_manager_dialog::PopulateTrophyTable()
 	m_trophy_table->setSortingEnabled(false); // Disable sorting before using setItem calls
 
 	std::shared_ptr<rXmlNode> trophy_base = data->trop_config.GetRoot();
-	if (trophy_base->GetChildren()->GetName() == "trophyconf")
+	ensure(trophy_base);
-	{
-		trophy_base = trophy_base->GetChildren();
-	}
-	else
-	{
-		gui_log.error("Root name does not match trophyconf in trophy. Name received: %s", trophy_base->GetChildren()->GetName());
-		return;
-	}
 
 	QPixmap placeholder(m_icon_height, m_icon_height);
 	placeholder.fill(Qt::transparent);
@@ -895,7 +890,7 @@ void trophy_manager_dialog::PopulateTrophyTable()
 		}
 
 		// Get data (stolen graciously from sceNpTrophy.cpp)
-		SceNpTrophyDetails details;
+		SceNpTrophyDetails details{};
 
 		// Get trophy id
 		const s32 trophy_id = atoi(n->GetAttribute("id").c_str());

rpcs3/rpcs3qt/trophy_manager_dialog.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "Utilities/rXml.h"
+#include "Loader/TROPUSR.h"
 
 #include <QWidget>
 #include <QComboBox>
@@ -22,7 +22,7 @@ class TROPUSRLoader;
 struct GameTrophiesData
 {
 	std::unique_ptr<TROPUSRLoader> trop_usr;
-	rXmlDocument trop_config; // I'd like to use unique but the protocol inside of the function passes around shared pointers..
+	trophy_xml_document trop_config; // I'd like to use unique but the protocol inside of the function passes around shared pointers..
 	std::unordered_map<int, QPixmap> trophy_images; // Cache trophy images to avoid loading from disk as much as possible.
 	std::unordered_map<int, QString> trophy_image_paths;
 	std::string game_name;