Datapack tags always replace, even when they should append
YanisBft opened this issue ยท 26 comments
API version: 0.1.4.71
Description:
My mod adds new walls. So I have to add these new walls to the vanilla block tag minecraft:walls.
With a "false" value in the "replace" field, my walls should append to the vanilla ones. But instead, they replace them.
They don't, works just fine for me. Can you show debugging/screenshots or anything as evidence?
See the image below:
On the left, the vanilla wall does not have "#minecraft:walls" block tag anymore.
On the right, my mod wall does have it, so the two walls can't connect to each other.
EDIT: You can also test my mod and see the issue directly (https://minecraft.curseforge.com/projects/fabriblocks)
I tested both @asiekierka
As I didn't say it before, it works fine in dev environment. The issue comes when using the mod after the build.
Just ran into this issue as well. When adding vanilla tags to blocks, in the dev env, and when using "replace": "false",
it works, but when using that and not in dev env, it does not work.
Using:
minecraft_version=1.14.2 Pre-Release 4
yarn_mappings=1.14.2 Pre-Release 4+build.1
loader_version=0.4.8+build.154
fabric_version=0.3.0-pre+build.167
My current solution to this is to add the vanilla blocks to my own vanilla tags, but that is not really maintanable
I tested this with the mod Desire Paths using desire-paths-1.2.0.jar
When the mod is active, bamboo saplings cannot be planted on grass or sand blocks, it can only be planted on desire-paths blocks.
The file bamboo_plantable_on.json inside the jar contains this:
{
"replace": false,
"values": [
"desire-paths:grass_dirt_inter",
"desire-paths:mycelium_dirt_inter",
"desire-paths:podzol_dirt_inter",
"desire-paths:dirt_coarse_inter"
]
}
If the file is edited like this:
{
"replace": false,
"values": [
"desire-paths:grass_dirt_inter",
"desire-paths:mycelium_dirt_inter",
"desire-paths:podzol_dirt_inter",
"desire-paths:dirt_coarse_inter",
"minecraft:grass_block",
"minecraft:sand"
]
}
Then bamboo saplings can be planted on grass and sand blocks.
So it seems that "replace": false is not being honored.
It was mentioned on Discord that this happens only in production and not in dev env.
My testing on api 0.4.0 shows that this issue applies depending on some variable. The tags vanilla tags missing in eclipse but when mod is compiled and on an actual client there are no issues.
Using gradlew runclient it also replaces:
This happens with bamboo_plantable_on.json as such:
{
"replace": false,
"values": [
"desire-paths:grass_dirt_inter",
"desire-paths:mycelium_dirt_inter",
"desire-paths:podzol_dirt_inter",
"desire-paths:dirt_coarse_inter",
"minecraft:bamboo",
"minecraft:bamboo_sapling",
"minecraft:gravel",
"#minecraft:sand",
"#minecraft:dirt_like"
]
}
With bamboo_plantable_on.json as such:
{
"replace": false,
"values": [
"desire-paths:grass_dirt_inter",
"desire-paths:mycelium_dirt_inter",
"desire-paths:podzol_dirt_inter",
"desire-paths:dirt_coarse_inter"
]
}
so this may either be resource loader bug or loom bug. last time it was loom not loading vanilla resource when mod resource is present.
I've noticed no change after applying @Chocohead 's fix. Also in my case, it doesn't just "replace", but rather only vanilla blocks added to tags are recognized on the debug screen as containing the tag, but not by crafting recipes or by the clear
command. Mod blocks that are supposed to contain the tag do not have it at all, for all I can see. This is the same with or without the "fix".
Seems to still be happening on latest snapshot/fabric api. Tried adding my blocks to the new piglin item / block data tags but once loaded only my blocks / items are in that tag despite replace being false.
Note: I haven't tested chocoheads fix.
Edit: Issue only happens in dev.
tag works- the โfalseโ parameter doesnโt work correctly. you must put the value in quotes.
Could this be fixed? I rely on tags to test things in my mod and it's impossible to see what it should actually be if the tags are replaced. Thanks!
This problem still exists for
minecraft_version = 1.16.3
loader_version = 0.9.3+build.207
fabric_version = 0.21.0+build.407-1.16
This is a classpath ordering problem, although a much more subtle one than expected:
Firstly, it's important to remember KnotClassLoader
operates as an inverse delegating classloader. That is the resources (and therefore classes) it knows about will come before anything on the parent classloader. The reason behind this is to allow Mixin to be applied to all the classes it is aware of, at the expense of the chance for accidents of course.
Next is to look at how Tag
s load. RegistryTagManager
is responsible for reloading tags whenever a new save is loaded, each tag type is handled by its own RegistryTagContainer
. The relevant loading to this problem happens in TagContainer#prepareReload
's lambda. It will search for all the JSONs which are in the appropriate data asset directory, then load the declarations the resource manager is aware of for each distinct tag name. These declarations are found via ReloadableResourceManagerImpl#getAllResources
, which will use NamespaceResourceManager#getAllResources
for the appropriate namespace a particular tag name comes from. For example, #minecraft:logs
will search all declarations of tags/blocks/logs.json
in the minecraft
namespace. From here each ResourcePack
(which has at least one asset in the namespace) will handle providing what they have, and it is here that the culprit is made clear. The class to blame for all this is DefaultResourcePack
.
DefaultResourcePack
is the ResourcePack
responsible for the asset in the Minecraft jar. Its subclass DefaultClientResourcePack
is also responsible for searching the asset index for anything the jar doesn't have (such as sounds, which are handled fine coincidentally). Because Minecraft is designed around being the only asset provider properly on the classpath it makes some sweeping presumptions about where it can find these assets. All asset loading is done either via DefaultResourcePack.class.getResource
or DefaultResourcePack.class.getResourceAsStream
. Both of which happen to be completely dependent on KnotClassLoader
's choice for which resource it picks.
But Choco, you say, the first jar KnotClassLoader
knows about is always the GameProvider
(ie the Minecraft jar). And for that you are right. Yet there is now two complexities to this problem. The first is what happens if the jar doesn't contain a resource yet another mod does, and the second to why tag loading is broken in dev.
The first complexity is simple, the asset is duplicated. The default pack claims to have it, but will return an identical stream to the real ModNioResourcePack
(or indeed just leak something out of a mod jar even if it didn't have its own resource pack given).
The second complexity, is brought about from the Mixin 7495ea2 added for DefaultResourcePack
. It goes through every duplicate instance of an asset on the classpath (through KnotClassLoader
reverse delegating) then presumes that the last instance is also from the Minecraft jar. Which for me is true, unless I happen to change the classpath ordering. Then it isn't. So if my mod assets are last, they will appear to be the Minecraft jar's assets and load instance (very similar to the first complexity). My assets will only be last of course if they're on the boot classpath to start with, which is only true when an IDE has put them there. Hence a dev only problem.
It is worth mentioning, perhaps as an aside, that the Mixin fix also happens to crash out DirectoryResourcePack#isValidPath
when a resource is from a jar rather than shadow Minecraft's check in DefaultResourcePack#isValidUrl
. DefaultResourcePack
might have changed since it was added, either way it's why found
is outside of the try block (and why there's two tries to start with).
The fix I came up with is to replace MixinDefaultResourcePack
with
@Mixin(DefaultResourcePack.class)
abstract class DefaultResourcePackFix {
@Shadow
private static @Final Map<ResourceType, FileSystem> typeToFileSystem;
@Inject(method = "findInputStream", at = @At("HEAD"), cancellable = true)
private void onFindInputStream(ResourceType resourceType, Identifier identifier, CallbackInfoReturnable<InputStream> callback) {
if (DefaultResourcePack.resourcePath != null) {
// Fall through to Vanilla logic, they have a special case here.
return;
}
FileSystem fs = typeToFileSystem.get(resourceType);
if (fs == null) return; // Apparently Minecraft couldn't find its own resources, they'll be an error in the log for this
Path path = fs.getPath(resourceType.getDirectory(), identifier.getNamespace(), identifier.getPath());
if (path != null && Files.isRegularFile(path)) {
try {
callback.setReturnValue(Files.newInputStream(path));
} catch (IOException e) {
// Something went wrong, vanilla doesn't log these errors though
}
}
}
@Inject(method = "contains", at = @At("HEAD"), cancellable = true)
private void fixContains(ResourceType type, Identifier id, CallbackInfoReturnable<Boolean> callback) {
if (DefaultResourcePack.resourcePath != null) return;
FileSystem fs = typeToFileSystem.get(type);
if (fs == null) return;
Path path = fs.getPath(type.getDirectory(), id.getNamespace(), id.getPath());
if (path != null) callback.setReturnValue(Files.isRegularFile(path));
}
}
Haven't tested it outside of dev but I can't give you all the answers in one night :P The first complexity is not entirely fixed as DefaultResourcePack#openRoot(String)
might find a resource from another URL
if not present in the Minecraft jar, but fixing that would require actually finding the real jar ourselves rather than using what Mojang helpfully already provided.
The ability to finely control internal resource pack ordering is blocking for an issue on my mod magneticflux-/fabric-diagonal-panes#15. I think this issue is closely related, and could be solved at the same time if we clean up resource loading a bit more.