Skip to content

Conversation

@AxBolduc
Copy link

As I explained in #145 the luaL_loadbufferx detour that lovely uses doesn't get hit for games that use LOVE versions before 11.5 (In my case 11.4). Instead of luaL_loadbufferx love.dll calls luaL_loadbuffer which has a very similar signature.

This is a pretty crude implementation that uses the windows API to get the version number on love.dll and uses that to determine which detour to use for the game at runtime.

Super open to feedback as I'm not sure that this is the best solution.

Copy link
Owner

@ethangreen-dev ethangreen-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @AxBolduc, thanks for the PR!

You're definitely on the right track but you can simplify things by calling love.version instead of scraping the module's PE header via win32. (https://github.com/love2d/love/blob/1462bd47cf222885c182878950d3f3c10ad92380/src/modules/love/love.cpp#L288).

@AxBolduc
Copy link
Author

@ethangreen-dev Thanks for finding that function for me, definitely better than using the win32 apis. I took a crack at calling the function from the dll and this currently works. Let me know if there was a better way for me to make the external call I just used the logic that we use to load the luaL_loadbuffer function.

@ethangreen-dev
Copy link
Owner

ethangreen-dev commented Feb 24, 2025

You can copy how we dynamically load lua51 functions in the sys module (It's alright to add the libloading dependency to lovely-win):

pub static LUA_LIB: Lazy<Library> = Lazy::new(|| unsafe { Library::new("lua51.dll").unwrap() });
Keep it in the windows crate as we'll figure out other platforms in a different PR.

@AxBolduc
Copy link
Author

@ethangreen-dev Took another crack at it. Let me know if there's anything else we want to clean up.

@ethangreen-dev
Copy link
Owner

@AxBolduc Sorry for the delay, we've had our hands full with other tasks. This PR should be moved over to use the functionality implemented in #198. If complete on time I think that this will fit nicely into the merge window for the next release.

@WilsontheWolf
Copy link
Collaborator

Is this PR still relevant? #305 seems to implement a similar change, not sure if it fixed the issue this was meant to close or not.

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.

3 participants