Restore vanilla's gametest structure loading

[SquidDev]

If a structure cannot be found, we now fall back to vanilla's codepath, rather than throwing an exception. This offers the following advantages:

• Makes it easier to use game tests in multi-loader setups, as the structure path is consistent across loaders.
• As structures are now looked up from testStructuresDirectoryName, it's now possible to store structures in plain text (.snbt) rather than their binary form (.nbt).

[modmuss50]

it's now possible to store structures in plain text (.snbt)

• The existing code only supported using snbt files.

• With this change a missing structure will throw a misleading error about the fallback location.

• The testmod should be expanded to cover this.

I wonder if it would be better to offer an API that allows mods to customise how the structures are loaded? The vanilla location for test structures is less than ideal IMO, allowing mods to have full control over this seems like an even better solution to me?

[SquidDev]

The existing code only supported using snbt files.

Ahh, you're entirely right. I think I had registered this at some point, but forgot by the time I came to write the commit message. Apologies!

With this change a missing structure will throw a misleading error about the fallback location.

Would you prefer a more complete implementation which tries the three paths and lists the paths it tried?

I wonder if it would be better to offer an API that allows mods to customise how the structures are loaded? The vanilla location for test structures is less than ideal IMO, allowing mods to have full control over this seems like an even better solution to me?

Initial disclaimer here that this may be in part just a documentation/understanding issue. I'm fairly sure I understand how the vanilla/Forge gametest workflow works, but still trying to grok what Fabric has (and hasn't) changed.

While vanilla's default location definitely isn't perfect, testStructuresDirectoryName isn't final, which means mods can overwrite it to point to a more sensible location. More crucially, /test export saves to this directory, which makes the test workflow much more convenient than having to shuffle files about after exporting.

My concern with an API is that once you start looking in custom locations, you break the symmetry between the /test import|export commands and the actual gametest structure loading. I'm not sure - happy to implement whatever API if it is wanted though!

[modmuss50]

More crucially, /test export saves to this directory, which makes the test workflow much more convenient than having to shuffle files about after exporting.

This is a great point, the current solution does add a bit more friction for developing these tests. The easier they are to add, the more likely people are going to adopt them. (I dont think many people use them right now)

Its been a little while since I looked at this stuff so I think I'll need to familiarise my self before I can give a fully informed suggestion.

[SquidDev]

Looking at this patch again, it occurs to me that falling back to vanilla behaviour isn't correct either, as the template name now contains the mod id, so this will look up files in the form modid:test_name.snbt instead.

I wonder if the following set of changes is a little more preferable:

• Make this mixin now search in Paths.of(testStructuresDirectoryName, structureId.getPath() + ".snbt") and then inside the datapack.
• Support setting testStructuresDirectoryName via a fabric-api.gametest.structure-dir system property.

Mods can then set this property to src/testmod/resources/data/$modid/gametest/structures inside their Gradle config. This is a little clunky, so I'm not too sure about it, but it does preserve. the existing layout and behaviour but with the existing advantage that the /test commands work as expected.

[apple502j]

I believe the proper way, 1.19+, is to register StructureTemplateManager.Provider instead of Mixin.

[SquidDev]

I guess the problem with StructureTemplateManager.Provider is that it also doesn't provide the paths it has tried. I don't know how much that matters?

[Technici4n]

Superseded by #2801.