Add RunE function
Hey @faiface , I was hacking a bit on Pixel and wanted to clean the code of my new game a little. One step here is currently error handling since I would like to avoid panic but instead have a clean shutdown and logging in place. Also for testing, it is much nicer to not rely on panics but have a well defined error handling in place.
To do this I would like to pass a function that returns an error to pixelgl.Run(…) (or actually a new function pixelgl.RunE(…) ). The change in mainthread makes this possible.
If this PR gets merged I will open a follow up PR at pixel to add a RunE function there :sunglasses:
Cheers Friedrich
This is reasonable, however, two comments:
-
RunErrwould be a better name and more consistent with the rest of the functions. - Avoid code duplication. Implement
RunErrusingRunor vice versa.
Thanks for the feedback. I did some more playing with this new functionality and I'm not 100% sure myself if this change is actually a good idea. I suppose it all depends if there is an actual benefit of allowing users of this library of running anything outside of mainthread. At least for my use case in pixel, while I could do that, there is not really a strong use case do do so because I can still simply do:
func main() {
pixelgl.Run(func() {
conf := loadConfig()
err := run(conf)
if err != nil {
panic(err) // TODO: better error handling
}
})
}
If this change gets merged however, it will complicate the package and maybe make it harder to be used because people might wonder which function they should now actually use. I would like to sit on it a little longer and maybe even close the PR. Happy to hear your thoughts if you have an opinion.
I implemented your suggestions (i.e. renaming the function to RunErr(…) and reduced code duplication.
Up to you if you find this addition to the library useful or not. I won't take any offense if you don't want to include this new function since I'm myself still a bit undecided if the use case for it is actually strong enough :thinking:
You call :)