MemoryWidget: Add 'Offset' and General Improvements

[SirMangler]

Summary of changes:

• Added a new field for Search Address Offset
• Fixed a hard crash when trying set a value while the core is not running.
• Brushed up the UI and spacing

The primary change here is adding an Offset field next to Search Address. All this does is add the offset to the supplied search address. While this sounds mediocre on paper, this is incredibly convenient for hack and cheat developers (and especially reverse engineers) who frequently work with base addresses and classes. I have been working on PrimeHack for nearly a year, and I constantly need to work with addresses that are offset from dynamically allocated classes in memory. Adding this simple Offset field streamlines traversing through large classes in memory rather than the otherwise clunky process.

The Offset field is resizeable and can be totally collapsed if wanted, but it really doesn't get in the way of the regular Search Address. The Find Next/Find Previous buttons work as expected, they simply clear the offset field if it finds a result.

Before/After

[shuffle2]

fwiw ctrl+k,ctrl+d is "format document" in VS (also a named action in vscode / shift+alt+f)

[AdmiralCurtiss]

Not sure if I fully understand how the offset should be used, but otherwise looks fine to me. I could see extracting the 'calc address from GUI fields' code to a separate function so it's not there three times, maybe? But not a huge deal.

[Rumi-Larry]

All groups have a header, except for the dumping buttons. I suggest adding one saying "Memory Dump"

[Rumi-Larry]

Also, the "Search" group should be closer to the actual search bar. Like after the "Set Value" button, the "Find Next" and "Find Previous" buttons should be right after, making the "Search" header unnecessary.

[SirMangler]

All groups have a header, except for the dumping buttons. I suggest adding one saying "Memory Dump"

the "Search" group should be closer to the actual search bar.

Not bad suggestions!

Not sure if I fully understand how the offset should be used

Say hypothetically you have a class at address 8000000. At 8000004 you have it's health. 8000008 you have ammo. 800000C you have some random array. You can just do the 8000000 as the address, and in offset you put in 4 for health, 8 for ammo, C for the array.

Now sure that's easy to do in your head and easily change the search address field. But what if we use more real numbers, you have a function loading the address 805C72AB into r4, and an instruction loading 0x241(r4) (which means 805C72AB + 0x241 if you didn't know) into r5, and another instruction loading 0x14c(r4) into r6 (aka 801C72AB + 0x14c).

It does an operation, r5 + r6. You want to figure out the answer to that? You have to copy the register r4 into a calculator, add 0x241 to that, and paste the result into the memory search address field. Then once you note that value, you would have to copy register r4 again (most likely from an open notepad somewhere), paste into your calculator and add 0x14c. Paste that into the memory search address field and note that answer down.

Or with this new PR? Copy r4 into the memory search address field. Copy 0x241 into the offset and note the value. Copy 0x14c into the offset and note the second value.

I have to do this process a lot for a variety of different scenarios, this feature while very simple, would be a pretty big time saver. No more needing to store the address somewhere and open a calculator every time I need to calculate an offset without doing it in my head. Especially useful if you're trying to map out a class and have to go between several offsets to see what is doing what.

In the end, you can ignore it if you want, you can even collapse it so its hidden.

[JMC47]

Yeah, using offsets is something I see in a lot of other debugging GUIs and I believe it would be a nice feature to have in Dolphin's debugger too.

[AdmiralCurtiss]

So while I'm not 100% happy with some of this, this works fine for what it's intended to do, so I don't really have any objections to just merging it as-is even if those are not addressed. The debugger needs more work anyway.

[SirMangler]

Is this okay to be merged? It seems all specific issues and comments have been resolved.

[leoetlino]

nitpick, but there's no point in repeating the type name here:

Suggested change

	QSplitter* m_address_splitter = new QSplitter(Qt::Horizontal);
	auto* m_address_splitter = new QSplitter(Qt::Horizontal);

[leoetlino]

Suggested change

	bool good_addr, good_off;
	bool good_addr, good_offset;

off isn't a particularly common abbreviation for offset, and you use offset below, so I'd prefer to see it written out.

[leoetlino]

Suggested change

	QFont addr_font, off_font;
	QFont addr_font, offset_font;

[leoetlino]

Suggested change

	QPalette addr_palette, off_palette;
	QPalette addr_palette, offset_palette;

[leoetlino]

Suggested change

	u32 addr = m_search_address->text().toUInt(&good_addr, 16);
	const u32 addr = m_search_address->text().toUInt(&good_addr, 16);

[leoetlino]

Suggested change

	u32 offset = m_search_offset->text().toUInt(&good_off, 16); // Returns 0 if conversion fails
	// Returns 0 if conversion fails
	const u32 offset = m_search_offset->text().toUInt(&good_offset, 16);

[leoetlino]

ditto

[leoetlino]

The comment should be on its own line.