This ticket presents a solution rather than a problem, so it is always a bit tricky to look through that.

I am not a fan of using 0x8000 marker to detect patch-packs. For JGRPP it holds true, but JGRPP is not the only patchpack out there. There are several others, which use other ways to fiddle with their savegame versions. The block you reference shows this clearly. So if we would be to use 0x8000 it is specially for JGRPP, not for any patchpack.

That said, under this ticket is of course the question: when you load a savegame from a patchpack (any patchpack), it says it is made with a newer version. Which strictly seen is true, and as such, not a bug. But we could maybe be nicer about these things.

What I have been thinking for a while, that maybe we should include a string in the savegame to indicate what "version" it is. We set it to "vanilla", JGRPP can set it to "JGRPP", and any other patchpack to what-ever the want to be called. That means that if we spot a mismatch, we can tell where the savegame came from.

The other approach is to upstream some parts of the savegame system JGRPP uses, which allows us to detect if there are blocks we don't understand. That also makes it possible to give a more clear error.

All in all, there is a bit of investigation that needs to be done here, to solve the question: "can we better tell users when they are using the wrong patch-pack".

But, using 0x8000 is a no for me. As always, we have to keep the broader view, and not a narrow one :)

In case it's not clear -- the quoted comment including For future patchpack creators: please follow a system like JGRPP, where the version is masked with 0x8000 is already present in OpenTTD's saveload.h (since d8c8f4e).

It seems strange to 'officially' recommend this approach and then decline to support it. Also, JGR's has been by far the most-used patchpack for several years, so there will be many existing savegames even if future versions switch to a hypothetical alternative.

That is fair; I guess I didn't write that part as a "official" recommendation, but your reasoning is sound to me. We have to be a bit careful, as it is fairly recent we added that (as you pointed out).

I still think that we should do better in making it more clear where savegames came from, but as I said, this ticket presents a solution, not a problem, which makes that a bit hard.

The current savegame header is a fixed 8 bytes in size, and doesn't include any spare bits which could be used to indicate "extended header follows", because of that there isn't an obvious place to put such a version string as suggested above.
You could make such a header conditional on the savegame version in the header, but you would need to handle the case where you're trying to load a savegame with a version number which suggests that the header should be there, but without such an extended header actually being present.

You could this in a chunk, but it's also worth noting that historically patchpacks have had different chunk headers which aren't ordinarily readable by trunk. (SpringPP and ChillPP both had this, my branch only has this for extra-large chunks).

For what it's worth more recent versions of my branch do include the complete version string in the extended version info chunk, specifically so that user of my branch get a friendlier error message when trying to load saves from an even newer version of my branch.
However you'd need to include a reasonably complete parser for the chunk format to get at it, which is probably not what you want.

Personally, I am not against vanilla being to read (and also use) your extended version info chunk, but I know that indeed it won't be trivial to get there :) It is just the more proper solution .. tnx for explaining this in high detail, much appreciated!