Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support resource pack filters, overlays and features from pack.mcmeta files in mods. #3312

Closed

Conversation

modmuss50
Copy link
Member

@modmuss50 modmuss50 commented Sep 11, 2023

Closes #3302

See: https://www.minecraft.net/en-us/article/minecraft-snapshot-22w11a for more info about filters.

Mods cannot filter each other, but they can filter packs (such as Minecraft its self) that are loaded before. Overlays and features are also supported.

@modmuss50 modmuss50 added the bug Something isn't working label Sep 11, 2023
@modmuss50 modmuss50 requested a review from a team September 11, 2023 13:27
@modmuss50 modmuss50 changed the title Support resource pack filters for mods. Support resource pack filters, overlays and features form pack.mcmeta files in mods. Sep 11, 2023
@modmuss50 modmuss50 changed the title Support resource pack filters, overlays and features form pack.mcmeta files in mods. Support resource pack filters, overlays and features from pack.mcmeta files in mods. Sep 11, 2023
@Technici4n
Copy link
Member

As suggested by Pepper, it would be best to "flatten" group resource packs instead.

@modmuss50
Copy link
Member Author

As suggested by Pepper, it would be best to "flatten" group resource packs instead.

Can you explain this a bit more, I must have missed the conversation? This PR is basically providing a "flatterned" pack.mcmeta file?

I think its very much out of scope to make large changes to this in this PR.

@Juuxel
Copy link
Member

Juuxel commented Sep 12, 2023

@modmuss50
Copy link
Member Author

The conversation is here on Fabricord: https://discord.com/channels/507304429255393322/566276937035546624/1150963915384160416

Pepper — Today at 02:19
There was some discussion in the NeoForge server today about a new implementation strategy for mod resource packs. It was suggested that the resource pack menu show one pack as it does currently, but that it would not itself provide any packs and only serve as a marker. Internally, each mod resource pack would be individually added to the ResourceManager pack list such that these packs take the place of the marker pack in the priority list. Having all packs be added separately would solve the issue of getting all resources for one location only retrieving the top-most resource from all mods and it would allow pack.mcmeta features to work automatically. Another benefit of separate packs I can add myself is that mod-added log warnings which report a pack ID will be more specific, as the pack ID will be "some_mod_resources" instead of "fabric_mods".

Posting here so its not lost, yeah that seems to make more sense at a high level actually doing it might be easier said than done. I dont know without trying, seems like it would be better suited to go in along with the larger scale resource loader changes.

For now I will park this PR.

@modmuss50 modmuss50 added the priority:low Low priority PRs that don't immediately need to be resolved label Sep 12, 2023
@maityyy
Copy link
Contributor

maityyy commented Sep 13, 2023

For now I will park this PR.

Why? This is an internal impl, does it somehow affect the behavior of these features?

@catter1
Copy link

catter1 commented Dec 5, 2023

Hey, with 1.20.3 coming out, could this please be revisted and made a higher priority? As a datapack developer, pack overlays not working on Fabric is a severe issue. Myself and many other creators I know of use overlays to have a single datapack for all 1.20.x versions.

In 1.20.2 this wasn't an enourmous issue, since the main thing that changed was potion NBT, and overlays are technically avoidable by adding both NBT formats at once.

However, 1.20.3 has the minecraft:grass to minecraft:short_grass change. In preparation for this, datapack creators are making an overlay to override the changes. If we want to package the datapack as a mod - which is extremely common - this functionality will not work, and the datapack will crash.

Yes, we could technically release make a separate 1.20.3 version of our datapacks to package as mods, and release separately. But this will just cause confusion having a version split in the mod releases, but not the datapacks. It will also be a lot more headache-inducing work for us. And as far as I'm aware, the overlay issue does not exist in Forge/NeoForged - making only Fabric hoding us back. (Please correct me if I'm wrong on this part.)

With Mojang making more and more breaking changes in minor version releases, overlays are our saving grace. We would greatly appreciate if this PR could be addressed and merged for 1.20.3. Of course backporting this to 1.20.2 as well would be very nice, but 1.20.3 is much more important at the moment.

@apple502j apple502j self-assigned this Dec 10, 2023
@apple502j apple502j removed the priority:low Low priority PRs that don't immediately need to be resolved label Dec 10, 2023
@apple502j
Copy link
Contributor

I am looking into this stuff, hopefully I can finish it before the end of this year. Not a guarantee though.

@lukebemish
Copy link

lukebemish commented Dec 14, 2023

I've been working on this over at NeoForged - this is the approach I took: neoforged/NeoForge#367 (this is what is meant by the "flattening" in question).

Basically, vanilla makes a couple of assumptions that the current system violates, and that's what leads to this issue in general. Those assumptions are (using yarn names):

  • one pack per ResourcePackProfile, in the form of a ResourcePackProfile.PackFactory
  • one copy of a resource at any given Identifier per ResourcesPack
  • ResourcePackProfile is the fundamental, separable unit of resource/data packs - multiple overlays become one ResourcePackProfile

Obeying these assumptions means that overlays will work, filters will work, any future such features Mojang adds will work, and any similar features mods add will work, all out of the box.

What these add up to, basically, is that flattening has to happen at the ResourcePackProfile level, not the ResourcePack level. The issue with an approach like the one this PR originally takes is that it could allow mod resource/data packs to sort of "bleed over" into one another - if multiple mods had the same overlay names or whatever, or if an overlay from one mod overlays a resource added by multiple mods like a tag file. If you decide it's the direction you'd like to go, feel free to steal whatever you'd like from the work I did in my PR to neoforged; I looked at implementing something similar for fabric a while back, and it looked feasible, though I ran out of time

@modmuss50
Copy link
Member Author

Superseeded by #3473

@modmuss50 modmuss50 closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pack.mcmeta functionality is not available
7 participants