Skip to content

Conversation

@glebm
Copy link
Collaborator

@glebm glebm commented Jul 19, 2025

Multiple Lua improvements:

  1. Follows advice from https://sol2.readthedocs.io/en/latest/functions.html to use set_function when binding functions.

  2. Adds autocomplete support for userdata methods, including handling . vs ::

    image image image
  3. Updates Lua and sol2 (with some patches for yet-to-be-merged PRs).

@glebm glebm marked this pull request as draft July 19, 2025 09:10
@glebm glebm force-pushed the sol-items branch 3 times, most recently from e2eaf3a to 3cce0d2 Compare July 19, 2025 14:20
@glebm glebm changed the title Lua items.cpp: Simplify property bindings Lua improvements Jul 19, 2025
@glebm glebm force-pushed the sol-items branch 2 times, most recently from 79802d1 to dac6d36 Compare July 19, 2025 14:29
@glebm glebm marked this pull request as ready for review July 19, 2025 14:34
@glebm glebm enabled auto-merge (rebase) July 19, 2025 14:34
@glebm glebm marked this pull request as draft July 19, 2025 16:24
auto-merge was automatically disabled July 19, 2025 16:24

Pull request was converted to draft

@glebm glebm marked this pull request as ready for review July 19, 2025 16:26
@glebm glebm enabled auto-merge (rebase) July 19, 2025 16:26
@glebm glebm force-pushed the sol-items branch 5 times, most recently from 1ac2a34 to 36cafb3 Compare July 20, 2025 08:57
@AJenbo AJenbo disabled auto-merge July 20, 2025 14:46
AJenbo
AJenbo previously approved these changes Jul 20, 2025
glebm added 5 commits July 20, 2025 15:47
Also renames lua/lua.hpp to lua/lua_global.hpp.

The previous name broke version auto-detection in sol2, which involves
calling `__has_include(<lua/lua.hpp>)`.
1. Follows advice from
   https://sol2.readthedocs.io/en/latest/functions.html to use
   `set_function` when binding functions.
2. Adds autocomplete support for userdata methods.
3. Simplifies property bindings and improves string handling.
Filter out more cases where autocomplete just gets in the way.
@AJenbo AJenbo merged commit 6fe4172 into diasurgical:master Jul 20, 2025
24 checks passed
@glebm glebm deleted the sol-items branch July 20, 2025 15:48
@tsunamistate
Copy link
Collaborator

tsunamistate commented Jul 21, 2025

@glebm patch application currently fails on Windows MSVC

[3/9] Performing patch step for 'sol2-populate'
patching file CMakeLists.txt
patching file documentation/CMakeLists.txt
patching file single/CMakeLists.txt
patching file include/sol/stack_field.hpp
patching file include/sol/usertype_container.hpp
patching file CMakeLists.txt
patching file CMakeLists.txt
patching file include/sol/reference.hpp
patching file include/sol/usertype_storage.hpp
patching file tests/regression_tests/1716/CMakeLists.txt
patching file 'tests/regression_tests/1716/source/1716 - registry indexing leak.cpp'
patching file tests/regression_tests/1716/source/main.cpp
patching file tests/regression_tests/CMakeLists.txt
Hunk #1 FAILED at 23 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file tests/regression_tests/CMakeLists.txt.rej
'true' is not recognized as an internal or external command,
operable program or batch file.
FAILED: sol2-populate-prefix/src/sol2-populate-stamp/sol2-populate-patch C:/src/DevilutionX/build/x64-Debug/_deps/sol2-subbuild/sol2-populate-prefix/src/sol2-populate-stamp/sol2-populate-patch 
C:\WINDOWS\system32\cmd.exe /C "cd /D C:\src\DevilutionX\build\x64-Debug\_deps\sol2-src && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0001-Reduce-cmake_minimum_required-to-3.22.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0002-Fix-array-index-out-of-bounds-in-stack_field.hpp.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0003-Change-end-to-sen-in-usertype_container.hpp.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0004-Fix-missing-Lua-Lua-target-when-using-system-Lua.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0005-fix-INTERFACE_LINK_LIBRARIES-property-for-lualib.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0006-Overload-stateless_reference_equals-and-stateless_re.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0007-Add-Nerixyz-s-test-and-fix-for-C-17.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0008-Faster-track-for-lightuserdata_value.patch || true ) && ( "C:\Program Files\Git\usr\bin\patch.exe" -p0 -N < C:/src/DevilutionX/3rdParty/sol2/patches/0009-Work-around-1581.patch || true ) && "C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E touch C:/src/DevilutionX/build/x64-Debug/_deps/sol2-subbuild/sol2-populate-prefix/src/sol2-populate-stamp/sol2-populate-patch"
ninja: build stopped: subcommand failed.

@AJenbo
Copy link
Member

AJenbo commented Jul 21, 2025

@tsunamistate make sure you have patch.exe installed in your system PATH so that cmake can find it.

@tsunamistate
Copy link
Collaborator

@AJenbo my patch.exe was successfully found by FindPatch.cmake from the Git installation, if you read the log, you can see that some of the files were patched correctly, and it failed due to:
Hunk #1 FAILED at 23 (different line endings).

@glebm
Copy link
Collaborator Author

glebm commented Jul 21, 2025

Oh, sorry about that. Can you try in the following order:

  1. Pass the input to patch via -i inputfile instead of < inputfile.
  2. Add --binary flag to the patch command.
  3. Change the line endings of the patch file.

If none of that works, I suppose we could fork a diasurgical/sol2 repo and have a branch with the patches that we want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants