MehVahdJukaar / Moonlight

GNU General Public License v3.0
50 stars 26 forks source link

[🐞]: Generation of dynamic resources for foreign namespaces doesn't work out of the box #178

Open Syst3ms opened 10 months ago

Syst3ms commented 10 months ago

BEFORE CONTINUING:

Version-Loader

1.20.1-fabric

Moonlight Lib Version

moonlight 1.20-2.8.57

Issue Detail

Still trying to add fluid tags for my mod's fluids, I stumbled upon an unrelated issue. It took me a ton of debugging to figure out the source of this one. Since vanilla fluid tags determine game physics, I wanted to add my mod's fluids to the vanilla minecraft:water and minecraft:lava tags, which I did with the following line of code (Kotlin), working around #177 :

fun DynamicDataPack.addFluidTag(builder: SimpleTagBuilder) {
    val loc = ResType.TAGS.getPath(builder.id.withPrefixedPath("fluids/"))
    addJson(loc, builder.serializeToJson(), ResType.GENERIC)
}

This worked like a charm for my own mod's tags, but it just did nothing for the vanilla tags. I checked, and the generated pack had the correct folder structure. I initially thought DynamicResourcePack was looking up resources wrong, and nearly filed a bug report for it, but the issue actually has to do with namespaces.

The constructor of LifecycledResourceManagerImpl (mojmap MultiPackResourceManager) starts as follows:

public LifecycledResourceManagerImpl(ResourceType type, List<ResourcePack> packs) {
    this.packs = List.copyOf(packs);
    Map<String, NamespaceResourceManager> map = new HashMap();
    List<String> list = packs.stream().flatMap(pack -> pack.getNamespaces(type).stream()).distinct().toList();
    // ...
}

It queries the namespaces for each resource pack, and the constructor dispatches the packs to NamespaceResourceManagers (mojmap FallbackResourceManager). The problem is, the namespaces of DynamicResourcePacks (besides the parent mod's) aren't determined until regenerateDynamicAssets fires, which only happens at the tail of this constructor, so this makes all registrations for namespaces other than the mod's ineffective.

Calling DynamicResourcePack#addNamespaces on init fixes the issue.

I'm filing a bug report because some bits of code lead me to believe this isn't intended behavior, specifically the first line of addBytes, which makes it seem like the namespaces are intended to be gathered dynamically:

protected void addBytes(Identifier path, byte[] bytes) {
        this.namespaces.add(path.getNamespace());
        this.resources.put(path, bytes);
        // ...
}

Log Attachment

N/A

OPTIONAL: To Produce

No response

OPTIONAL: Which mods are affected?

No response

MehVahdJukaar commented 10 months ago

Yes the namespace thing is annoying as Minecraft requires them to be ready before the resources are generated. You need to call add namespace call in your constructor with all the namespaces you are adding stuff to