TannerRogalsky / love.js

LÖVE ported to the web using Emscripten
MIT License
491 stars 52 forks source link

Using pcall to catch an exception generates an exception in the release build #28

Closed TannerRogalsky closed 8 years ago

TannerRogalsky commented 8 years ago

The release build is compiled with exceptions disabled. As such, code that expects an exception to be thrown will likely error when none is in the release build.

Discovered here: https://github.com/love2d-community/splashes/issues/11

TannerRogalsky commented 8 years ago

Resolved by providing a compatibility build with exceptions enabled.

s-ol commented 8 years ago

IMO this is a core Lua feature and needs to work in the non-compat build aswell (as opposed to shader feature set fixes and other compatibility layers)

TannerRogalsky commented 8 years ago

@S0lll0s I'm open to suggestions. Only thing I can think of would be to actually rewrite Love to not use exceptions and that would be a lot of work.

s-ol commented 8 years ago

Hm, im a bit confused. You say rewrite LÖVE, bht pcall and lua errors are provided by luajit, no?

Also luajit is a C library and therefore can't throw exceptions if I am not mistaken.

Or does löve use a C++ wrapper around the luajit bindings that uses exceptions? If so, that would maybe narrow the code that needs to be changed down enough to be doable, either way I need to have a look at the source.

s-ol commented 8 years ago

Okay, a quick glance at the luajit info page reveals that luajit has built-in C++ extension support whilst being ABI-compatible to regular C-Lua. I suppose that means that the code involving exceptions in LÖVE is pretty far-spread in the source and hard to replace with a different error handling mechanism :/

Isn't there a way to "compile exceptions away" with emscripten? If it is common practice to disable exceptions in emscripten, surely other people had issues like this?

also, have you done performance tests? How much worse is love.JS with exceptions enabled? (But all other optimizations enabled)

TannerRogalsky commented 8 years ago

Ah, I wasn't clear. pcall and errors generated in Lua work fine in every release. It is only errors generated in C++ that are suppressed.

Consider this code: https://bitbucket.org/rude/love/src/d17873cede8586e23dbf4b121b87efd823bd658c/src/modules/joystick/sdl/JoystickModule.cpp?at=default&fileviewer=file-view-default#JoystickModule.cpp-44

SDL_InitSubSystem(SDL_INIT_JOYSTICK | SDL_INIT_GAMECONTROLLER) will return -1 on platforms where joysticks are not supported which which Love expects to generate an exception and halt execution of that function. In performant emscripten code, that exception is not generated and the execution does not halt so then Love tries to use elements of the joystick subsystem despite the fact that it wasn't initialized.

Technically, pcall is working but Love is in an undefined state. Does that make sense?

s-ol commented 8 years ago

Sorry for not getting back at this, I see. Still haven't looked at the löve source though, the real problem only is in places where exceptions are supposed to be thrown and bubble into Lua (like creating resources from nonexistent files), in which cases there would be an explicit throw usually... Still, changing the whole LÖVE source is not really a manageable way of keeping love.js up to date.

Did you ever talk this over with some of the main LÖVE devs like slime?

TannerRogalsky commented 8 years ago

Yeah, but the compat build solves this issue at the cost of some performance. I can't think of a middle ground between the compat build and the Love refactor so unless someone wants to step up and refactor Love to not use an exception-based model this seems to be our best bet for now.