techiew / DirectXHook

DirectX 11/12 hook including a simple overlay framework.
181 stars 23 forks source link

Some feedback #4

Closed Kaldaien closed 2 years ago

Kaldaien commented 2 years ago

I had to alter my own mod framework to ignore the dummy window + swapchain your mod creates during init (this is nothing you did wrong), and a few things came to my attention while reviewing your code.


  1. Loading DLLs from DllMain is iffy, particularly ReShade.

You might want to move that to your initialization thread, that'll also ensure that the vftable pointers you setup a few lines below it are not in a race with ReShade.



  1. The correct way to load a DLL using the system directory (as opposed to hard coding) is this:
HMODULE hModDXGIDebug =
    LoadLibraryExW ( L"dxgidebug.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32 );



  1. Do not call Sleep (...) on a thread that has a message pump (as in the pause code).

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-msgwaitformultipleobjects

if ( WAIT_OBJECT_0 == MsgWaitForMultipleObjects (0, nullptr, FALSE, 10UL, QS_ALLINPUT) )
{
  // A message is waiting
}

else
{
  // You timed-out
}

This way you can keep the application responsive while also not spinning a loop needlessly. The timeout can be set much higher than 10 ms, since the thread will wake up if there's input anyway.

techiew commented 2 years ago

Insane and embarrasing that Kaldaien reviewed my hook code, thank you for your feedback and I will be looking at it.

techiew commented 2 years ago

Thank you, I changed the code according to your feedback:

  1. I was originally having issues loading reshade.dll when I didn't load it inside dllmain, thus I loaded that one in dllmain also. But I think my loading of dxgi.dll (which had always been in dllmain) caused problems with loading of .dlls outside of dllmain. I also think this fact caused a bunch of other random inconsistent issues which I did not know the origin of. Loading the .dlls works fine now outside dllmain.
  2. I could not get LoadLibraryExW to work since it would cause the game to not load, I believe the reason being this fact which I found in the docs: "If the string specifies a module name without a path and more than one loaded module has the same base name and extension, the function returns a handle to the module that was loaded first."
  3. This was a great suggestion and it makes much more sense than the constant Sleep() I was calling. The window is more responsive now and you can actually tab back in to the window even if it is paused.

I love your work.