-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Added support to detect game installations by uninstaller location #18916
Conversation
This sounds reasonable, but can you please link to a specific example for this? I assume this is being implemented for a mod. |
Have you tested that this does not break the existing use case? The documentation at https://docs.microsoft.com/en-us/dotnet/api/system.io.path.getdirectoryname?view=net-5.0 suggests that it will. /*
This code produces the following output:
...
GetDirectoryName('C:\MyDir\MySubDir') returns 'C:\MyDir'
...
*/ |
I think a better approach here would be to rename |
ee74b36
to
5398161
Compare
Yes, I agree that will be safer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine to me, but one more simplification inline below. We have had several bugfixes in the installation logic recently, so can you please also rebase this on the latest bleed?
5398161
to
7fa7dad
Compare
Updated as instructed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but not tested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also untested, but lgtm anyway
This is for cases where no
InstallFolder
is available, but the uninstaller executable location is written to the registry and it is located in the game directory, which is quite common place in the 90s.