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

Allow datagen to use vanilla armor trim textures #146

Merged

Conversation

GizmoTheMoonPig
Copy link
Contributor

1.20 introduced armor trims, which use dynamically generated textures for the armor overlay based on what material is used. Unfortunately, since these textures don't exist until the game is actually run, there's not an easy way to add textures like these to item models, as the ModelBuilder's texture method checks that the texture actually exists.
This PR aims to fix that in an easy way, by adding a new uncheckedTexture method to the ModelBuilder. Using this method won't make the ModelBuilder check if the texture actually exists when generating an item model, allowing you to add things like trim overlays to your armors using datagen.
I chose to name it uncheckedTexture because models use a similar naming convention with UncheckedModelFile, which doesn't check if a model actually exists when using it in datagen. If anyone has a better name proposal, please let me know and I'll change it.
I also added to the existing DataGeneratorTest to show that the method works as intended. Switching uncheckedTexture to just use texture will throw an error since the texture doesn't actually exist when it's called.

@ChampionAsh5357
Copy link
Contributor

Honestly, I don't think this is a bad solution since we have UncheckedModelFile. However, the correct solution to this would be to update ExistingFileHelper to consume the generated runtime resources.

@GizmoTheMoonPig
Copy link
Contributor Author

I'll have to look into that then. We're entering an area I'm not too familiar with so it may take me a little bit to get familiar with it. Will mark as a draft until I get this fully figured out.

In order to switch to changing the ExistingFileHelper instead I need to make sure that this will also work with any generated resources mods make as well, not just vanilla. I'm sure there's mods out there that have their own systems to do this, or some may use vanilla's system in the future at some point.

@GizmoTheMoonPig GizmoTheMoonPig marked this pull request as draft September 25, 2023 04:17
@PlatinPython
Copy link
Contributor

Yeah, the correct solution, imo, would be to use ExistingFileHelper#trackGenerated for those textures. I'm doing that in my mod and it works great.

@GizmoTheMoonPig
Copy link
Contributor Author

Yeah, the correct solution, imo, would be to use ExistingFileHelper#trackGenerated for those textures. I'm doing that in my mod and it works great.

Thank you for letting me know about this! I'm very glad I didn't have to cobble something up myself.
I've changed the PR to instead add vanilla trims to the ExistingFileHelper in the ItemModelBuilder by default, since apparently mods can just add their stuff as needed. I considered using the trim material registry for this but that would require a lot more tinkering that I don't really think is necessary, so it's using a hardcoded list of vanilla trim materials at the moment. I've removed the uncheckedTexture method now that this is done a different way, and the test mod has been updated to show that it works even when using the normal texture method again.
I'll keep this marked as a draft just in case this isn't the way I should be going about this. Once someone from triage gives their approval, I'll unmark it.

@GizmoTheMoonPig GizmoTheMoonPig changed the title Allow datagen to use textures that dont actually exist yet Allow datagen to use vanilla armor trim textures Sep 25, 2023
@GizmoTheMoonPig GizmoTheMoonPig marked this pull request as ready for review October 5, 2023 04:18
@sciwhiz12 sciwhiz12 added enhancement New (or improvement to existing) feature or request 1.20 Targeted at Minecraft 1.20 labels Nov 1, 2023
@GizmoTheMoonPig
Copy link
Contributor Author

I've updated this PR to 1.20.2, feel free to look it over whenever :)

@sciwhiz12 sciwhiz12 merged commit 01da808 into neoforged:1.20.x Nov 4, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20 Targeted at Minecraft 1.20 enhancement New (or improvement to existing) feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants