Use StaticVector for all vendors

[Yggdrasill]

Hello,

So now that #7906 got merged, I figured it was time to rebase and update this PR too. This PR will use the StaticVector class for all the town vendors which fixes some buffer overflow bugs by using a memory safe structure. I have wanted to merge the test suite in #7906 first, as it ensures that the changes in this PR didn't break anything.

Rationale

A while ago I noticed a buffer overflow when starting a new game, which was fixed by #7875. The source of this problem is that the vendor item arrays were shrunk by one recently. This caused a buffer overflow as SortVendor() (and multiple other functions) rely upon sentinel values at the end of these arrays.

Since arrays that depend on sentinel values share similar problems to C-strings, I think it's a good idea to move away from that. Originally I was intending to simply use a struct with an array and a size field, but @ephphatha suggested to use StaticVector. It is a good fit for the vendor item arrays in particular since their size is known at compile-time, and vectors provide a very convenient interface for the purpose.

Changes made

This PR redefines all the vendor item arrays as StaticVector types, and makes relevant changes to the SpawnVendor functions. Multiple functions in stores.cpp are changed and logic that uses sentinel values has been removed in favour of the size() method. It also redefines items.cpp:SortVendor() and stores.cpp:ScrollVendorStore() to take an std::span<Item> type, and the test cases in vendor_test.cpp are updated to use StaticVector.

Issues fixed

This actually does fix some rare crashes in the game as well that I've experienced while buying non-pinned items from Adria. While I haven't bisected like I did for the SortVendor() fix, I highly suspect the same commit is the reason for it. My suspicion is that in WitchBuyItem() (and probably SmithBuyItem(), for the same reason) the game crashes on these lines:

	// WitchBuyItem()
	if (idx >= 3) {
		if (idx == NumWitchItemsHf - 1) {
			WitchItems[NumWitchItemsHf - 1].clear();
		} else {
			for (; !WitchItems[idx + 1].isEmpty(); idx++) {
				WitchItems[idx] = std::move(WitchItems[idx + 1]);
			}
			WitchItems[idx].clear();
		}
	}

	// SmithBuyItem()
	if (idx == NumSmithBasicItemsHf - 1) {
		SmithItems[NumSmithBasicItemsHf - 1].clear();
	} else {
		for (; !SmithItems[idx + 1].isEmpty(); idx++) {
			SmithItems[idx] = std::move(SmithItems[idx + 1]);
		}
		SmithItems[idx].clear();
	}

If SpawnWitch() or SpawnSmith() max roll the amount of items, there is no space for a sentinel value at the end of the array. This results in writes out of bounds, causing a segmentation fault on my system. While I haven't verified this for sure in a debugger, I am quite convinced that is the source of these crashes. This PR resolves these crashes by not relying on any sentinel values as far as the vendor item arrays go, instead using the size() method.

Item generation

Item generation is identical between this PR and the master branch, and the tests written verify this. You can also force the item generation seed, and the same items will be generated between the branches.

Conclusion

This will fix any issues related to buffer overflows in the vendor item arrays, and cleans up some fairly messy code in the various VendorBuyItem functions. It also lays a foundation for further cleaning up stores.cpp and the various SpawnVendor functions.

Regards,
yggdrasill

[AJenbo]

Thanks, this reads a lot cleaner and it's nice not to have all the dodgy size checks and better crash guarding.