Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lua integration for Hellfire and modding #7397

Closed
wants to merge 3 commits into from

Conversation

kphoenix137
Copy link
Collaborator

This PR moves all Hellfire logic to lua scripts, and allows for lua mods to be loaded from an external filepath.

Modders are able to utilize the following:
Pre-hook: Allows running Lua code before the C++ code.
Post-hook: Allows running Lua code after the C++ code.
Override: Completely overwrites the function with a provided Lua function.

Hellfire scripts are located in Source/lua/hellfire, and there is a script for each cpp file that contains Hellfire specific logic.

@kphoenix137
Copy link
Collaborator Author

kphoenix137 commented Sep 3, 2024

@StephenCWills https://github.com/diasurgical/devilutionX/pull/7397/files#diff-d3f2f653697b88b01c5f94e7a52c2a16db00e2a26d5d9ab504ea75feef090311R191

When running the hellfire/player.lua script, I encounter a crash because of the handling here. My assumption is that if this function only had setters and getters along with the existing member functions, this would work correctly.

@StephenCWills
Copy link
Member

When running the hellfire/player.lua script, I encounter a crash because of the handling here. My assumption is that if this function only had setters and getters along with the existing member functions, this would work correctly.

According to the code examples I've seen, member variables should work fine. There's probably some other reason. Maybe something to do with the build check failure.

  Error: D:\a\devilutionX\devilutionX\Source\lua\lua.cpp(194): error C2672: 'sol::state_view::new_usertype': no matching overloaded function found
  D:\a\devilutionX\devilutionX\build-ninja-vcpkg-relwithdebinfo\_deps\sol2-src\include\sol/state_view.hpp(784): note: could be 'sol::basic_usertype<T,sol::reference> sol::state_view::new_usertype(Args ...)'
  D:\a\devilutionX\devilutionX\Source\lua\lua.cpp(194): note: 'sol::basic_usertype<T,sol::reference> sol::state_view::new_usertype(Args ...)': could not deduce template argument for 'Args &&' from 'overloaded-function'

My recommendation is don't throw every single thing in there all at once. Make it work with a subset of the members and add more gradually.

@kphoenix137
Copy link
Collaborator Author

When running the hellfire/player.lua script, I encounter a crash because of the handling here. My assumption is that if this function only had setters and getters along with the existing member functions, this would work correctly.

According to the code examples I've seen, member variables should work fine. There's probably some other reason. Maybe something to do with the build check failure.

  Error: D:\a\devilutionX\devilutionX\Source\lua\lua.cpp(194): error C2672: 'sol::state_view::new_usertype': no matching overloaded function found
  D:\a\devilutionX\devilutionX\build-ninja-vcpkg-relwithdebinfo\_deps\sol2-src\include\sol/state_view.hpp(784): note: could be 'sol::basic_usertype<T,sol::reference> sol::state_view::new_usertype(Args ...)'
  D:\a\devilutionX\devilutionX\Source\lua\lua.cpp(194): note: 'sol::basic_usertype<T,sol::reference> sol::state_view::new_usertype(Args ...)': could not deduce template argument for 'Args &&' from 'overloaded-function'

My recommendation is don't throw every single thing in there all at once. Make it work with a subset of the members and add more gradually.

I found the problem. It was registering a template function.

@kphoenix137 kphoenix137 closed this Sep 3, 2024
@kphoenix137
Copy link
Collaborator Author

kphoenix137 commented Sep 3, 2024

Unfortunately, my changes are causing issues that make the compiler run out of heap space. I'm just not entirely sure what I'm doing with this whole Lua integration, so I'm closing this to leave the door open for someone more experienced to take a crack at it.

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.

2 participants