[lua] restore rollback of failed transactions

[lampysprites]

Hello! This hopefully fixes a sneaky problem that, regretfully, was created by me a while ago when I needed to fix a different issue.

Currently, calling error() from inside app.transaction in lua script does not rollback changes. If that happens the transaction seems to "slurp" all following drawing and commands. On the surface undo/redo keeps working, except undo history window which gets stuck on the last item in the list before running the script. Sometimes, depending on script doings, it may also modify pixels that are not recorded by history states.

My understanding is that an early return (removed in te commit) prevented disposal of the Tx for some reason (?).

With this change, the transaction will roll back, which I have tested with some scripts that were failing for me, and also confirmed by logging TX_TRACE.

Regarding catching errors that happen inside transaction, cases where an actual error happens

local a, b = "foo", "bar"
app.transaction("Test1", function() 
    print(a + b)
end)

app.transaction("Test1", function() 
    error("it broke")
end)

will revert transaction correctly, and still print the error message, while

app.transaction("Test1", function() 
    error()
end)

will revert it silently, so an option to cancel on purpose remains.

---

I agree that my contributions are licensed under the Individual Contributor License Agreement V4.0 ("CLA") as stated in https://github.com/igarastudio/cla/blob/main/cla.md

I have signed the CLA following the steps given in https://github.com/igarastudio/cla#signing

[aseprite-bot]

clang-tidy review says "All clean, LGTM! 👍"

[dacap]

Hi @lampysprites, thanks for this PR. I've just tested it and it looks like the bug is still present even with this fix. I'll try to find a solution right now (I want to release a new version with some fixes).

Probably this is a new behavior with some latest change to the Tx handling (some changes were introduced in the latest version to avoid several crashes running scripts + backing up data for recover in case of crash).

[lampysprites]

Welp guess it's more complex that it looks. This (and tx logging) was sole change on a fresh clone though. Thanks for your efforts~

[dacap]

Thank you @lampysprites, I'm checking and the issue might be solved only compiling Lua as C++, to enable stack unwinding (calling all destructor correctly) when an error happens. Working on it 👍

[dacap]

@lampysprites I've just pushed a fix for this in the main branch, you can give it a try. This problem should be fixed for the next release.

[lampysprites]

It doesn't seem to link without including lua with "extern C" in vc22, because of mangled names.. although cmakelists looks right to me?

CMakeCache if matters...

Also, removing that one lua_error entirely would be acceptable tradeoff to me. It does not break the logic of transaction, only confusing part is that errors aren't visible unless caught, printed and rethrown manually.

[dacap]

Hmm 🤔 probably you have to regenerate the whole solution. I don’t use the IDE (compiling with Ninja here even with MSVC 2022). Cmake should regenerate the solution automatically so I’m not sure, but the flags to compile Lua were updated to compile it as C++ code (and avoid those errors). At least the github actions CI looks like it’s linking on windows.

[lampysprites]

It was aseparate repo of course o(≧口≦)o
The problem is with cmake generated projects ig. Ninja builds everything (with cl.exe I presume), and the fix works. In vc /TP flags do not apply for some reason, and I neede to manually change "Compile As" in project settings for lua/lualib/lauxlib, then it linked:

Anyway the fix works, thanks a lot!