search

momentum-mod / lumper

BSP lump editor and review tool
MIT License
23 stars 14 forks source link

Add error handling to BSP loading process #14

Closed tsa96 closed 1 month ago

tsa96 commented 1 year ago

Currently we have several general error-handling TODOs sitting around in the code, especially for BSP file loading. This can easily end up stagnating, we should work on it ASAP. MainViewModel.IO's has saving but not loading. This was actually making debugging harder for XB earlier, seemed like an internal Lumper thing but really just file path stuff. This should be caught at displayed in a message box.

Also, BspFile's Load(string path) has a nasty looking "// TODO: loads of error handling" at the top, @BIRD311 what are plans for this?

BIRD311 commented 1 year ago

Loading in the UI does work and you wrote the "// TODO: loads of error handling" in BspFile 🙃. But yes, error handling todos should be cleaned up. Console errors should also be changed, so they show up in GUI and/or console in case we what a CLI version in the future.

tsa96 commented 1 year ago

Oh apologies, nonetheless my old Todos def want removing if done or addressing if still relevant.

File loading was broken on Windows for strings with whitespace and nothing was being caught, error in core was being ignored by Avalonia so trying to open a file simply had no effect. Presumably it's best to leave the exceptions handled by lumper core to be handled by CLI and UI (maybe catch exception in core then rethrow some custom LumperIOException or something), unless we want to integrate some kinda stdout/stderr interface to the UI, is it really good practice to have lumper core wire to stderr as its really just a library? Why not just just document errors it can throw and have consumers handle them?