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

Add static methods for loading images #23782

Closed
KoBeWi opened this issue Nov 17, 2018 · 8 comments
Closed

Add static methods for loading images #23782

KoBeWi opened this issue Nov 17, 2018 · 8 comments

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Nov 17, 2018

Right now, if you want to assign an arbitrary image as a texture, you need to do this:

var image = Image.new()
image.load("some/path.png")
var texture = ImageTexture.new()
texture.create_from_image(image)
$Sprite.texture = texture

This is a lot of code for simple operation. I know that load is supposed to return error code, but it can be done smarter. There could be static versions of both load and create_from_image, which return null if load fails (as simple as that). The above code could then become this:
$Sprite.texture = ImageTexture.create_from_image(Image.load("some/path.png"))
I know this is unsafe code, but there are people like me who don't mind unsafe code. Well, the unsafeness could be mitigated at all if trying to create from null image would result in a warning instead of error and simply return null.

There are probably more classes that would benefit from more static methods btw.

@TheCire
Copy link

TheCire commented Nov 17, 2018

I'm not sure but I think the variant system "automagically" turns an image into a texture behind the scenes. I have code like this

var texture = load("path/to/image.png")
$Sprite.texture = image

and it works just fine like that...

@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 17, 2018

Yeah, but it works like this only if the texture is imported as a part of your project. If you want to load external texture, you need to use the code I provided.

@TheCire
Copy link

TheCire commented Nov 17, 2018

Gotcha, what about just creating your own wrapping function then?

func load_texture(path):
  var image = Image.new()
  image.load(path)
  var texture = ImageTexture.new()
  return texture

add it to a global singleton script or however you want to do it :)

Yeah, I get your point about having a built-in static function, but does that go along with the coding style and the way the engine works?

@Aaron-Fleisher
Copy link

@TheCire Power of GDScript! I love doing mini wrapper functions like that. I feel the OP's sentiments however, sometimes I wish there was an easier way to do something, but then realize you can really achieve a lot of stuff in GDS by itself

@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 18, 2018

Well, I know I can make a mini-wrapper (I actually did that). But then I have to copy it over to all projects I'd need it. Also I can't just have utility script with everything, because I don't always need all these functions and it's project-specific a lot >_>

My proposition seems logical and fixes the issue without workarounds, but maybe I'm just used to having language work that way.

@TheCire
Copy link

TheCire commented Nov 18, 2018

Looking at your code from your OP @KoBeWi , looks like Image should have a static initializer like so:

var image = Image.load("path/to/image.png")

instead of having to do:

var image = Image.new()
image.load("path/to/image.png")
#etc

@PetePete1984
Copy link
Contributor

#20972
The last time this came up, reduz was in favor of removing Image.load entirely

@KoBeWi
Copy link
Member Author

KoBeWi commented Jul 14, 2020

There seem to be little interest in this and I actually needed it in just one or two projects, so closing.

There's a slightly related proposal though godotengine/godot-proposals#1101

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

No branches or pull requests

5 participants