ControllerEngine: do not use C++ assertions for JS issues

[Be-ing]

Using a C++ debugger for JavaScript issues is weird, not really
helpful, and annoying.

[Be-ing]

Error handling has already been improved in main since #2682. I will not put any more effort into hacking around that in 2.3. Using C++ assertions instead is IMO inappropriate, very annoying, and a significant regression from 2.2.

[Holzhaus]

significant regression from 2.2

How is it a regression if it's opt-in?

Removing this requires mapping developers to always keep an eye on the terminal output. I admit that debug assertions isn't the best way to handle this, but we should at least thow a JS error in that case.

[Be-ing]

we should at least thow a JS error in that case.

This is already implemented in main. I will not spend any effort to backport it. I'm not sure if we could even get the behavior we want from the legacy QScriptEngine.

[Be-ing]

How is it a regression if it's opt-in?

It makes --controllerDebug unusable in case the mapping has harmless errors.

[Be-ing]

--controllerDebug is a cludge that couples too much functionality together. It should be removed in favor of proper use of logging categories.

[Be-ing]

This will be a major support issue if we don't merge this before releasing 2.3.0.

[uklotzde]

This will be a major support issue if we don't merge this before releasing 2.3.0.

Assertions won't be enabled in release builds. Only an issue in beta and snapshot builds.

[Be-ing]

It will still be an issue for users of 2.3 building from source.

[Holzhaus]

It will still be an issue for users of 2.3 building from source.

If they enable assertions.

[Be-ing]

Anyway, lots of people have voiced support for this. It's very annoying. I don't think there's any reason to hold back merging any longer.

[Holzhaus]

Can we keep the assertions for tests? If you looks at the controllerdebug method, it returns true when testing or when --controllerDebug is passed. We should keep the assertions in the former case until we have a suitable replacement. We had bugs in our midi-components library for months and nobody noticed...

[Be-ing]

I'm not investing any more effort to implement nonessential improvements for a system I'm simultaneously working on replacing.

[Be-ing]

Creating even more merge conflicts to resolve with main is a bad idea IMO.

[Be-ing]

@Holzhaus if you want you can make tests fail if a JS error is thrown in main after this is merged. But backporting that to 2.3 would be practically pointless IMO.

[Be-ing]

merge?

[Holzhaus]

@Holzhaus if you want you can make tests fail if a JS error is thrown in main after this is merged. But backporting that to 2.3 would be practically pointless IMO.

The point is that nonexisting/invalid controls don't cause a JS error, they just print a warning to the CLI. But anyway, let's merge this even if it means a testing regression. Mappings are user data after all, and the current behaviour is hacky.

[uklotzde]

Who is able to resolve the merge conflicts 2.3 -> main?

[Be-ing]

I will work on it. Merging #3667 to main is nontrivial though.