`loadTexture` should throw errors

[define-private-public]

I'm mainly talking about the loadTexture() proc here: https://github.com/zacharycarter/zengine/blob/master/src/zengine/texture.nim#L3, but it could also apply to the one that takes in an openArray.

Could you have that proc actually throw errors instead of returning nil? It would be a bit more handy for logging and errors when it comes to loading spritesheets.

1. If it could throw a FileNotFoundError if it can't find the file that would be great. I think we need a Custom Exception for that since it doesn't exist in the standard library.
2. Another custom error for "Wasn't able to load the texture." That would give me flexibility to tell the user that a file was found, but it wasn't a valid texture/image.

Do we have a list of valid image formats? I assume we're using SDL_Image's image loading. What are it's limitations?

[define-private-public]

Uses of loadTexture thus far are here:

zengine/models.nim 265: material.texDiffuse = loadTexture(filename) 272: material.texNormal = loadTexture(filename) 277: material.texSpecular = loadTexture(filename)

zengine/text.nim 230: defaultFont.texture = loadTexture(imagePixels, imgWidth, imgHeight)

[zacharycarter]

We are using SDL_Image and right now it's mainly limited by zengine's own image loading code found here : https://github.com/zacharycarter/zengine/blob/master/src/zengine/zgl.nim#L440-L456

I think SDL_Image is going to support most formats we'll encounter, but there may be some it doesn't. I just don't have any list handy.

I'm a bit wary of introducing any kind of exceptions into the code base. The loadTexture method already warns the user when a texture file can't be found vs when it can't be loaded. Can we solve this some other way? If not we can introduce exceptions until I can think of a better way to handle this.

[define-private-public]

My other idea is that the proc could return a tuple. The first value being the texture (or nil), and the second being an error code (or zero if successful). I think errors are nice they do add extra info if something went wrong.

[zacharycarter]

I'm playing devil's advocate here : If we add exceptions then we have to write exception handling code in at least four places, plus any future places where we reference this procedure. Also - if the procedure itself is already warning the caller of what is wrong via warnings - what benefit does returning an exception provide?

I agree that getting specifics about what is wrong is nice, but I tend to avoid exceptions in the pursuit of keeping my code simple and not making the caller handle a lot of edge situations. If you think that it adds benefit, I'm fine with it.

[define-private-public]

I can understand how exception handling code can be a bit of a pain in the ass to write. I only really have two counter pointers:

1. Those warning messages are for logging only; correct? What if a user/programmer turns off logging then distributes their program and then encounters an error. There won't be a logged message of what went wrong. So when the user comes back to the developer for help, the dev is going to have a hard time figuring out what went wrong. Errors also provide traces, which is quite the handy thing.

2. The calling code can choose what errors to handle¸ and how to handle them. Let's say that I try to load the texture, but if the texture file isn't found, then I'll want to create a new texture and save it to that location. If the proc produced a FileNotFound, and not a UnableToLoadTexture, then I would know that I'm good to create a new texture and save it to that location. But If I got the UnableToLoadTexture, I'd know that isn't really a safe place to write the new texture (as I wouldn't want to overwrite any data).

I guess you could summarize it up as more verbose and safer. I'll go with what you want, but my hat is in for throwing errors (and adding handling where it makes sense). It'd be easier to do this while the code is still young rather then when it's bigger.

[zacharycarter]

Will add these in shortly.

[zacharycarter]

Closed with ef073416e3dd59bf52d872b44512edfb76219b3c