chunky icon indicating copy to clipboard operation
chunky copied to clipboard

Refactor `Texture`.

Open ThatRedox opened this issue 1 year ago • 2 comments

ThatRedox avatar Sep 01 '24 07:09 ThatRedox

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.

leMaik avatar Sep 06 '24 19:09 leMaik

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

ThatRedox avatar Sep 06 '24 19:09 ThatRedox