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

Refactor Texture. #1755

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

ThatRedox
Copy link
Member

No description provided.

# Conflicts:
#	chunky/src/java/se/llbit/chunky/resources/Texture.java
#	chunky/src/java/se/llbit/chunky/resources/TexturePackLoader.java
@leMaik
Copy link
Member

leMaik commented Sep 6, 2024

This PR makes it so that Texture isn't even a texture anymore but just the list of hard-coded textures. Should we instead have an interface (or abstract class) Texture and then let texture types inherit from that?

The texture loaders and SimpleTexture (which isn't a texture but a texture loader, but that's outside of this PR's scope) now require BitmapTexture. Is that an actual requirement or would it be sufficient to require Texture? Maybe we should refactor the texture loaders from taking all parameters and a target Texture object to having a method that returns the texture. That way, the loader could select an appropriate Texture implementation.

@ThatRedox
Copy link
Member Author

This PR makes it so that Texture isn't even a texture anymore but just the list of hard-coded textures. Should we instead have an interface (or abstract class) Texture and then let texture types inherit from that?

Turning Texture into an interface might be a better solution. Originally I thought not changing texture would be less work (which wasn't the case at all).

The texture loaders and SimpleTexture (which isn't a texture but a texture loader, but that's outside of this PR's scope) now require BitmapTexture. Is that an actual requirement or would it be sufficient to require Texture?

The texture loaders need BitmapTexture to store the loaded texture into something, since not all textures support this (or should support it).

Maybe we should refactor the texture loaders from taking all parameters and a target Texture object to having a method that returns the texture. That way, the loader could select an appropriate Texture implementation.

This would probably be a better solution, but I'm not sure how it'd play out with having the list of hard coded textures. It'd probably involve a bigger refactor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants