-
Notifications
You must be signed in to change notification settings - Fork 19
Zleub/texture provider #402
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this is looking like a good solution to our texture needs, and should also help with some of the motr needs as well (duplicating minecraft textures so that translucency/alpha can be applied).
| sourceSets.main.resources { srcDir 'src/generated/resources' } | ||
|
|
||
| processResources { | ||
| duplicatesStrategy = DuplicatesStrategy.EXCLUDE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we expecting duplicates in the long term?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I got this added at the point where I had both textures, the original and the generated one. It seems that gradle copies over the generated ones when running the game and fail if no duplicate strategy is in place. There's only one discord message from mcjty that address this issue so I feel like we should keep it somehow even commented out
Idk how you want to handle that, let me know
| this.textureOutput = textureOutput; | ||
| } | ||
|
|
||
| public void tintGenerator(ResourceLocation sourcePath, ResourceLocation destinationPath, int color) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little thing, but I would suggest generateTinted or createTinted or even just tint - something a little more verb like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, makes sense
| public void registerTextures(TextureGenerator textureGenerator) { | ||
| processorBlockColorMap.forEach((number, color) -> { | ||
| // Make all processor blocks textures | ||
| Arrays.stream(TextureVariant.values()).toList().forEach((variant) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably could skip the toList in this.
| import java.awt.Color; | ||
|
|
||
| public class TintTransform extends TextureTransform { | ||
| Color tint; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True
| public ResourceLocation sourcePath; | ||
| public ResourceLocation destinationPath; | ||
|
|
||
| TextureTransform(ResourceLocation sourcePath, ResourceLocation destinationPath) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better public unless there is a good reason to have it package-private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to have those derived in another place than the datagen\texture folder ? If no then package private is fine no ?
| ResourceMetadata resourceMeta = input.metadata(); | ||
| AnimationMetadataSection textureMeta = resourceMeta.getSection(AnimationMetadataSection.TYPE).orElse(null); | ||
|
|
||
| InputStream r = input.open(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to ensure this is closed after use, either via a try-with-resources or an explicit close in a finally block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, as the java doc stated, this stream is closed after use by ImageIO.read (doc). I guess the edge case would be it to be null, which is going to be catch
|
Oops, just realised this was still in draft, sorry >_< |
|
Hey, no worries, I'm not the best at writing idiomatic java so those review are always welcomed |
|
@Zleub Is there anything more you were looking to do with this? I see it is still a draft. |
Hello, this is a new provider for the mod, texture provider. The aim is to allow quick texture derivation as shown for processor blocks and water texture.
This is a proposition to get #295 out of the way without tweaking anything from a FluidBlock.