Rust-SDL2 / rust-sdl2

SDL2 bindings for Rust
MIT License
2.76k stars 473 forks source link

Please don't panic! #692

Open dagit opened 7 years ago

dagit commented 7 years ago

I was looking through the operations for Texture and I noticed at least one call to panic!.

I'd like to advocate that as a library, it's probably always preferable to percolate errors like that to the caller, even if it represents a serious error. It would create another breaking API change, but I would certainly prefer to deal with that error at the application level rather than have my application abort. If it were returned as a Result, my application may still need to terminate ASAP but I would have the opportunity to exit gracefully with an additional context for the user, possibly saving data, etc.

Thanks.

Cobrand commented 7 years ago

I think some panic of SDL2 can definitively be replaced by unreachable macros. However some API calls like with Texture occur when there is a driver error, and that's true we could replace some of them by proper Err variants.

I'd like to redo the whole error handling as a whole before 1.0 anyway, because some techniques are way out of date with current rust idioms. We have super nice libraries such as error-chain now, I think we could use something like that. However the project is so huge, I'm leaving that for 1.0 for now (which should be the "final" version of this crate hopefully).

dagit commented 7 years ago

I'd like to help out if I can. What's a good way to discuss the design? Should I prepare a PR and then discuss it using github's code review feature?

Thanks!

Cobrand commented 7 years ago

If you think you have a good enough level in Rust to create a PR that would be merged on the first try, then go ahead and open a PR! I might have some comments for your PR, but I think it will be fine overall.

If you're unsure about some things, or want assistance because you're new, then I'm available on Mozilla's IRC (rust and rust-gamedev channels) on the evenings PST.

Thanks for the help by the way!