[1.21] Resource Conditions in Pack Overlays #3872
Conversation
|
It looks like the conditionalrecipes game tests are failing. 🤔 |
|
Fixed the failing gametest by moving the conditional overlays test into a proper gametest. |
.../src/main/java/net/fabricmc/fabric/api/resource/conditions/v1/OverlayConditionsMetadata.java
Outdated
Show resolved
Hide resolved
.../src/main/java/net/fabricmc/fabric/api/resource/conditions/v1/OverlayConditionsMetadata.java
Outdated
Show resolved
Hide resolved
.../src/main/java/net/fabricmc/fabric/api/resource/conditions/v1/OverlayConditionsMetadata.java
Outdated
Show resolved
Hide resolved
...v1/src/main/java/net/fabricmc/fabric/mixin/resource/conditions/ResourcePackProfileMixin.java
Outdated
Show resolved
Hide resolved
| * limitations under the License. | ||
| */ | ||
|
|
||
| package net.fabricmc.fabric.impl.resource.conditions; |
There was a problem hiding this comment.
Why isn't this a public API class? Without this class pack.mcmeta datagen is impossible
There was a problem hiding this comment.
Oops, I see this was asked to be changed a month ago — in my defence, the PR kept getting closed and it was a bit hard to keep track of.
In any case I still think this class should be an API anyway
There was a problem hiding this comment.
We should definally be able to datagenerate this, however I worry just moving this to API isnt the ideal solution.
- Generally we dont make records/data classes part of the API, we would have an interface + static factory function, it provides a better API surface.
- How easy is it to datagenerate the pack.mcmeta file right now? I dont think we do anything special for it, this does sound like a useful thing to have. Maybe its best leaving this as-is for now and making a nice API for generating the file?
There was a problem hiding this comment.
Generally we dont make records/data classes part of the API
However, this class is very similar to (even copies) the vanilla PackOverlaysMetadata, which makes it easy to understand how to use it
How easy is it to datagenerate the pack.mcmeta file right now?
The only thing is that ResourceFilter requires AW for BlockEntry::<init>, but that's the only such case. I datagen pack.mcmeta to filter recipes & advancements
(yes, another TAW candidate)
There was a problem hiding this comment.
Well, actually, the static factory function sounds ok, the main thing is to give a name similar to the vanilla class (yarn)
There was a problem hiding this comment.
Going to merge this PR as-is, we can have a follow up PR making this better for datagen.
…/overlay-conditions-1.21 # Conflicts: # fabric-resource-loader-v0/src/main/java/net/fabricmc/fabric/impl/resource/loader/ResourceManagerHelperImpl.java
Supercedes #3702, rebased to target 1.21.