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

Cyberpunk: forced load libraries instead of RB #163

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ZashIn
Copy link
Contributor

@ZashIn ZashIn commented Sep 13, 2024

Remove RootBuilder support/requirement (for CET and RED4ext) in favor of forced load libraries:
CET and RED4ext will be installed normally via USVFS instead into root folder for RB.

Here is a test release for the current MO v2.5.2.

This needs to be tested on larger mod lists for stability!
Additional discussion: CDPR-Modding-Documentation/Cyberpunk-Modding-Docs#37

TODO:

  • Dialog to convert existing mod lists using RB for CET and RED4ext
  • After release: update wiki with simpler installation
    • Add info for CrashReporter with CET and VFS
    • Remove / move RB requirement and instructions

Extras (in separate PR?):

  • Simplify load order handling, by changing defaults and notifying / converting old mod lists
  • fix profile specific save files (existing ones are not moved or similar)

@iarna
Copy link
Contributor

iarna commented Sep 27, 2024

I've done some testing with this:

The one weird thing I found is that if the game crashes, the crash handler then crashes itself, as it can't find version.dll:

image

My workaround for this is to make a symlink, eg, in the Cyber Engine Tweaks, going to bin/x64 and creating a CrashReporter folder and then from bin/x64/CrashReporter running mklink version.dll ../version.dll.

I wonder if this behavior is the cause of folks thinking it was a flaky approach?

@ZashIn
Copy link
Contributor Author

ZashIn commented Sep 29, 2024

My workaround for this is to make a symlink, eg, in the Cyber Engine Tweaks, going to bin/x64 and creating a CrashReporter folder and then from bin/x64/CrashReporter running mklink version.dll ../version.dll.

So you create the symlink in the CET mod folder? That would mean the additional mapping created by MO into the game folder does get catched up by the crash reporter, but the path resolution for ../version.dll is probably (without the symlink).

Since this is hard for me to reproduce, can you try to add an additional force load to your executable instead of the symlink:

  • CrashReporter.exe: ../version.dll (relative path might need to be different, I am guessing here)
  • CrashReporter.exe: ../winmm.dll

If this does not work, I can probably add an additional (custom) mapping as an alternative.

@iarna
Copy link
Contributor

iarna commented Sep 30, 2024

So you create the symlink in the CET mod folder?

Yup

image

That would mean the additional mapping created by MO into the game folder does get catched up by the crash reporter, but the path resolution for ../version.dll is probably (without the symlink).

Yes and no, the mapping applies to any process created by the process you originally launch, or something like that? I'm not super familiar with Windows process group dynamics, but emperically seems to be the case -- I'm using REDprelauncher to launch the game, and of course the mappings get passed through to so when Cyberpunk2077.exe does load it's gets the DLLs I told it about.

That the symlink fixes the CrashHandler.exe problem is a little weird to me, as yeah, either the VFS follows the symlink itself and makes CrashHandler.exe think version.dll is in its path, or it passes it through and CrashHandler.exe isn't have any problem reading "../version.dll". Which in either case raises the question of why it doesn't work without the symlink (seeing as it can clearly read the file through the VFS and it doesn't have this issue when manually installed).

I can't consistently trigger crashes either, but I'm gonna make the changes you suggested to my Force Load Settings and see if the problem recurs the next time a crash does occur.


That said, while thinking about this I found a new hypothesis as to why CrashHandler is crashing and why these changes may not help. And... what might actually help:

So, let's ask this question: How can CrashHandler.exe even fail to load "version.dll"? This is an important question because it's a core windows system dll -- that's why CET can use a wrapper for it to get itself loaded into memory. If CrashHandler.exe was loading it normally, it'd get it from C:\Windows\System32.

I think it was loading the DLL for stack trace purposes, and used the path that Cyberpunk2077.exe used. The injected DLL meant that it had no path, and so CrashHandler looked in its current path and found nothing. This'd also explain why it did not need a link to winmm.dll . Adding the symlink fixed that. Adding the force load... may or may not, depending on how they're loading the library.

If the force load config change fails, I'm going to try putting in a drive-absolute path, first with VFS location (ie \GoGLibrary\Cyberpunk2077\bin\x64\version.dll) and if that fails, one pointing at the mod (ie \Cyberpunk2077\CustomGame\mods\Cyber Engine Tweaks\bin\x64\version.dll).

Hoping that when CrashLoader loads the crash dump from CyberPunk it'll get the full path in the list of loaded DLLs.

@iarna
Copy link
Contributor

iarna commented Sep 30, 2024

Ok, I asked over on the modding discord about a way to crash the game on command and they came up with a CET command of GetPlayer():Dispose().

Testing was done after I removed my symlink. I'd wait till I got to the main menu, then brought up the CET console and ran the command above.

None of the forced load settings made any difference. I tried the following:

  1. Force load libraries for CrashHandler.exe of ..\version.dll and ..\winmm.dll.
  2. Force load libraries for CrashHandler.exe of E:\Cyberpunk2077\Intro\mods\Cyber Engine Tweaks\bin\x64\version.dll and E:\Cyberpunk2077\Intro\mods\Cyber Engine Tweaks\bin\x64\winmm.dll.

@iarna
Copy link
Contributor

iarna commented Oct 1, 2024

I wonder if it's possible to instruct MO2's VFS to provide version.dll in the CrashHandler folder without having to add an artifact to the installed mod?

CrashHandler does not appear to need access to winmm.dll or other injected DLLs, for whatever reason.

@ZashIn
Copy link
Contributor Author

ZashIn commented Oct 3, 2024

I tried adding an additional custom mapping to bin/x64/CrashReporter/version.dll, but the CrashReporter still does not find the lib, probably too fast for the VFS.
Also this and/or the additional force load can interfere with CET itself, so it is not an option.

I am not sure if we really want to add e.g. a symlink as a workaround just for this (and accept side effects like potential additional files created under CrashReporter - to be tested) or to just ignore it.

Does the CrashReporter actually provide anything (like debug info) for the user?

@iarna
Copy link
Contributor

iarna commented Oct 4, 2024

Ok, an even weirder followup:

  1. I tried copying Window's version.dll into CrashReporter -- it crashed with a mapping error
  2. I tried copying CET's version.dll into CrashReporter -- weirdly, the same crash as with the Windows copy.
  3. I tried hard linking CET's version.dll into CrashReporter --- and that worked.

(All my testing has been in mods that are deployed via VFS, along with the symlink variant.)

So apparently CrashReporter insists that its version.dll is actually the same file as CET's, not merely a copy of the same file.

@iarna
Copy link
Contributor

iarna commented Oct 4, 2024

Regarding the importance of CrashReporter --

It's output is only very rarely useful. It does include stack traces, and I have, once or twice, had that actually have something useful in it. (Exclusively from crashes happening during startup.)

My only concern about leaving things as they are would be that user's don't read docs and there will be a bug report and support impact.

TBH, if there's a way to disable CrashReporter I'd be inclined to advocate for that, with docs telling people how to enable it and ensure it continues working.

@iarna
Copy link
Contributor

iarna commented Oct 4, 2024

Tested with a no-op exe (literally void main () {}) replacing CrashReporter.exe and, unsurprisingly, it does silence the errors entirely.

The no-op CrashReporter.exe

@ZashIn
Copy link
Contributor Author

ZashIn commented Oct 4, 2024

Apparently you can even just replace it with an empty file, there is even a mod which does it like that.

@ZashIn
Copy link
Contributor Author

ZashIn commented Oct 6, 2024

@iarna disabling the CrashRepoter by adding a dummy mod with CET installed by default now.

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