Dimensions improve locks (maybe)

rpcs3/Emu/Io/Dimensions.cpp

@@ -77,10 +77,10 @@ void dimensions_toypad::generate_random_number(const u8* buf, u8 sequence, std::
 
 void dimensions_toypad::initialize_rng(u32 seed)
 {
-	random_a = 0xF1EA5EED;
+	m_random_a = 0xF1EA5EED;
-	random_b = seed;
+	m_random_b = seed;
-	random_c = seed;
+	m_random_c = seed;
-	random_d = seed;
+	m_random_d = seed;
 
 	for (int i = 0; i < 42; i++)
 	{
@@ -90,12 +90,12 @@ void dimensions_toypad::initialize_rng(u32 seed)
 
 u32 dimensions_toypad::get_next()
 {
-	const u32 e = random_a - std::rotl(random_b, 21);
+	const u32 e = m_random_a - std::rotl(m_random_b, 21);
-	random_a = random_b ^ std::rotl(random_c, 19);
+	m_random_a = m_random_b ^ std::rotl(m_random_c, 19);
-	random_b = random_c + std::rotl(random_d, 6);
+	m_random_b = m_random_c + std::rotl(m_random_d, 6);
-	random_c = random_d + e;
+	m_random_c = m_random_d + e;
-	random_d = e + random_a;
+	m_random_d = e + m_random_a;
-	return random_d;
+	return m_random_d;
 }
 
 std::array<u8, 8> dimensions_toypad::decrypt(const u8* buf, std::optional<std::array<u8, 16>> key)
@@ -292,7 +292,7 @@ void dimensions_toypad::get_challenge_response(const u8* buf, u8 sequence, std::
 
 void dimensions_toypad::query_block(u8 index, u8 page, std::array<u8, 32>& reply_buf, u8 sequence)
 {
-	std::lock_guard lock(dimensions_mutex);
+	std::lock_guard lock(m_dimensions_mutex);
 
 	// Index from game begins at 1 rather than 0, so minus 1 here
 	const dimensions_figure& figure = get_figure_by_index(index - 1);
@@ -311,7 +311,7 @@ void dimensions_toypad::query_block(u8 index, u8 page, std::array<u8, 32>& reply
 
 void dimensions_toypad::write_block(u8 index, u8 page, const u8* to_write_buf, std::array<u8, 32>& reply_buf, u8 sequence)
 {
-	std::lock_guard lock(dimensions_mutex);
+	std::lock_guard lock(m_dimensions_mutex);
 
 	// Index from game begins at 1 rather than 0, so minus 1 here
 	dimensions_figure& figure = get_figure_by_index(index - 1);
@@ -356,9 +356,12 @@ void dimensions_toypad::get_model(const u8* buf, u8 sequence, std::array<u8, 32>
 	reply_buf[12] = generate_checksum(reply_buf, 12);
 }
 
-u32 dimensions_toypad::load_figure(const std::array<u8, 0x2D * 0x04>& buf, fs::file in_file, u8 pad, u8 index)
+u32 dimensions_toypad::load_figure(const std::array<u8, 0x2D * 0x04>& buf, fs::file in_file, u8 pad, u8 index, bool lock)
 {
-	std::lock_guard lock(dimensions_mutex);
+	if (lock)
+	{
+		m_dimensions_mutex.lock();
+	}
 
 	const u32 id = get_figure_id(buf);
 
@@ -374,17 +377,27 @@ u32 dimensions_toypad::load_figure(const std::array<u8, 0x2D * 0x04>& buf, fs::f
 	std::memcpy(&figure_change_response[6], buf.data(), 7);
 	figure_change_response[13] = generate_checksum(figure_change_response, 13);
 	m_figure_added_removed_responses.push(figure_change_response);
+
+	if (lock)
+	{
+		m_dimensions_mutex.unlock();
+	}
 	return id;
 }
 
-bool dimensions_toypad::remove_figure(u8 pad, u8 index, bool save)
+bool dimensions_toypad::remove_figure(u8 pad, u8 index, bool save, bool lock)
 {
-	std::lock_guard lock(dimensions_mutex);
 	dimensions_figure& figure = get_figure_by_index(index);
 	if (figure.index == 255)
 	{
 		return false;
 	}
+
+	if (lock)
+	{
+		m_dimensions_mutex.lock();
+	}
+
 	// When a figure is removed from the toypad, respond to the game with the pad they were removed from, their index,
 	// the direction (0x01 in byte 6 for removed) and their UID
 	std::array<u8, 32> figure_change_response = {0x56, 0x0b, pad, 0x00, figure.index, 0x01};
@@ -398,11 +411,18 @@ bool dimensions_toypad::remove_figure(u8 pad, u8 index, bool save)
 	figure.pad = 255;
 	figure_change_response[13] = generate_checksum(figure_change_response, 13);
 	m_figure_added_removed_responses.push(figure_change_response);
+
+	if (lock)
+	{
+		m_dimensions_mutex.unlock();
+	}
 	return true;
 }
 
 bool dimensions_toypad::move_figure(u8 pad, u8 index, u8 old_pad, u8 old_index)
 {
+	std::lock_guard lock(m_dimensions_mutex);
+
 	if (old_index == index)
 	{
 		// Don't bother removing and loading again, just send response to the game
@@ -418,15 +438,15 @@ bool dimensions_toypad::move_figure(u8 pad, u8 index, u8 old_pad, u8 old_index)
 
 	// When moving figures between spaces on the toypad, remove any figure from the space they are moving to,
 	// then remove them from their current space, then load them to the space they are moving to
-	remove_figure(pad, index, true);
+	remove_figure(pad, index, true, false);
 
 	dimensions_figure& figure = get_figure_by_index(old_index);
 	const std::array<u8, 0x2D * 0x04> data = figure.data;
 	fs::file in_file = std::move(figure.dim_file);
 
-	remove_figure(old_pad, old_index, false);
+	remove_figure(old_pad, old_index, false, false);
 
-	load_figure(data, std::move(in_file), pad, index);
+	load_figure(data, std::move(in_file), pad, index, false);
 
 	return true;
 }
@@ -477,16 +497,16 @@ std::array<u8, 4> dimensions_toypad::pwd_generate(const std::array<u8, 7>& uid)
 
 std::optional<std::array<u8, 32>> dimensions_toypad::pop_added_removed_response()
 {
+	std::lock_guard lock(m_dimensions_mutex);
+
 	if (m_figure_added_removed_responses.empty())
 	{
 		return std::nullopt;
 	}
-	else
+
-	{
+	std::array<u8, 32> response = m_figure_added_removed_responses.front();
-		std::array<u8, 32> response = m_figure_added_removed_responses.front();
+	m_figure_added_removed_responses.pop();
-		m_figure_added_removed_responses.pop();
+	return response;
-		return response;
-	}
 }
 
 usb_device_dimensions::usb_device_dimensions(const std::array<u8, 7>& location)
@@ -517,11 +537,13 @@ void usb_device_dimensions::interrupt_transfer(u32 buf_size, u8* buf, u32 endpoi
 	transfer->expected_count = buf_size;
 	transfer->expected_result = HC_CC_NOERR;
 
-	if (endpoint == 0x81)
+	switch (endpoint)
+	{
+	case 0x81:
 	{
 		// Read Endpoint, if a request has not been sent via the write endpoint, set expected result as
 		// EHCI_CC_HALTED so the game doesn't report the Toypad as being disconnected.
-		std::unique_lock lock(query_mutex);
+		std::lock_guard lock(m_query_mutex);
 		std::optional<std::array<u8, 32>> response = g_dimensionstoypad.pop_added_removed_response();
 		if (response)
 		{
@@ -537,9 +559,9 @@ void usb_device_dimensions::interrupt_transfer(u32 buf_size, u8* buf, u32 endpoi
 			transfer->expected_count = 0;
 			transfer->expected_result = EHCI_CC_HALTED;
 		}
-		lock.unlock();
+		break;
 	}
-	else if (endpoint == 0x01)
+	case 0x01:
 	{
 		// Write endpoint, similar structure of request to the Infinity Base with a command for byte 3,
 		// sequence for byte 4, the payload after that, then a checksum for the final byte.
@@ -620,8 +642,12 @@ void usb_device_dimensions::interrupt_transfer(u32 buf_size, u8* buf, u32 endpoi
 			break;
 		}
 		}
-		std::lock_guard lock(query_mutex);
+		std::lock_guard lock(m_query_mutex);
 		m_queries.push(q_result);
+		break;
+	}
+	default:
+		break;
 	}
 }
 

rpcs3/Emu/Io/Dimensions.h

@@ -28,13 +28,13 @@ public:
 	void get_model(const u8* buf, u8 sequence, std::array<u8, 32>& reply_buf);
 	std::optional<std::array<u8, 32>> pop_added_removed_response();
 
-	bool remove_figure(u8 pad, u8 index, bool save);
+	bool remove_figure(u8 pad, u8 index, bool save, bool lock);
-	u32 load_figure(const std::array<u8, 0x2D * 0x04>& buf, fs::file in_file, u8 pad, u8 index);
+	u32 load_figure(const std::array<u8, 0x2D * 0x04>& buf, fs::file in_file, u8 pad, u8 index, bool lock);
 	bool move_figure(u8 pad, u8 index, u8 old_pad, u8 old_index);
 	static bool create_blank_character(std::array<u8, 0x2D * 0x04>& buf, u16 id);
 
 protected:
-	shared_mutex dimensions_mutex;
+	shared_mutex m_dimensions_mutex;
 	std::array<dimensions_figure, 7> m_figures;
 
 private:
@@ -50,10 +50,10 @@ private:
 	u32 get_next();
 	dimensions_figure& get_figure_by_index(u8 index);
 
-	u32 random_a;
+	u32 m_random_a{};
-	u32 random_b;
+	u32 m_random_b{};
-	u32 random_c;
+	u32 m_random_c{};
-	u32 random_d;
+	u32 m_random_d{};
 
 	u8 m_figure_order = 0;
 	std::queue<std::array<u8, 32>> m_figure_added_removed_responses;
@@ -72,6 +72,6 @@ public:
 	void isochronous_transfer(UsbTransfer* transfer) override;
 
 protected:
-	shared_mutex query_mutex;
+	shared_mutex m_query_mutex;
 	std::queue<std::array<u8, 32>> m_queries;
 };

rpcs3/rpcs3qt/dimensions_dialog.cpp

@@ -690,7 +690,7 @@ void dimensions_dialog::clear_figure(u8 pad, u8 index)
 
 	if (figure_slots[index])
 	{
-		g_dimensionstoypad.remove_figure(pad, index, true);
+		g_dimensionstoypad.remove_figure(pad, index, true, true);
 		figure_slots[index] = std::nullopt;
 		m_edit_figures[index]->setText(tr("None"));
 	}
@@ -755,7 +755,7 @@ void dimensions_dialog::load_figure_path(u8 pad, u8 index, const QString& path)
 
 	clear_figure(pad, index);
 
-	const u32 fig_num = g_dimensionstoypad.load_figure(data, std::move(dim_file), pad, index);
+	const u32 fig_num = g_dimensionstoypad.load_figure(data, std::move(dim_file), pad, index, true);
 
 	figure_slots[index] = fig_num;
 	const auto name = list_minifigs.find(fig_num);