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

Game does not associate with files on windows #14824

Closed
peppy opened this issue Sep 21, 2021 · 17 comments · Fixed by #27001
Closed

Game does not associate with files on windows #14824

peppy opened this issue Sep 21, 2021 · 17 comments · Fixed by #27001
Labels
platform/windows priority:1 Very important. Feels bad without fix. Affects the majority of users.

Comments

@peppy
Copy link
Sponsor Member

peppy commented Sep 21, 2021

Also it doesn't handle the IPC logic correctly to allow for such associations to work. This portion has been resolved via ppy/osu-framework#4783.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 3, 2024

This is pretty damn important for general usability, we should probably bump this to the top of the list after gameplay is in a good state.

@smallketchup82
Copy link
Contributor

smallketchup82 commented Jan 15, 2024

@peppy Do you plan on baking this into lazer itself or into the updater/installer? I wouldn't mind helping with this if its possible and not something only you can do.

I made an initial implementation (built into lazer) which would modify the system registry to associate the files & url handler with the lazer executable, but as I expected from the start, modifying HKCR requires administrator permissions. From my knowledge, elevating a program while its running isn't possible, and running lazer with admin permissions probably isn't a good idea. Then again I think that using ClickOnce file associations could be another way to go about it. Stable seems to do all of the file associations during installation. I'm kinda curious as to which approach you intend to take.

A plus of baking it into lazer itself is that you can press a button (e.g. in the maintenance section of the settings) to create the file associations, which would be useful to those who frequently switch between lazer and stable. At the same time, I don't believe this is possible since lazer runs at user-level.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 15, 2024

Stable elevates to UAC to set file permissions. Stable has no installer; it's all part of startup logic. To my knowledge the installer we use doesn't support file associations so it may be best for us to do it on startup in a similar way.

To avoid the game ever running in UAC mode itself, it should invoke a second instance, probably after letting the user know what's going on, via Process.Start.

If you're looking to work on this, it's best we outline exactly how it will work to avoid any wasted effort.

@peppy peppy added priority:0 Showstopper. Critical to the next release. priority:1 Very important. Feels bad without fix. Affects the majority of users. and removed priority:1 Very important. Feels bad without fix. Affects the majority of users. priority:0 Showstopper. Critical to the next release. labels Jan 15, 2024
@smallketchup82
Copy link
Contributor

smallketchup82 commented Jan 15, 2024

To my knowledge the installer we use doesn't support file associations so it may be best for us to do it on startup in a similar way.

Setting the associations during initial startup works. My only concern with this is making the action repeatable, since I believe that being able to trigger this manually could be useful to people down the line. I have two ideas on making this work. The first being that lazer can verify the integrity of the registry keys on startup, and if they look out of place, prompt the user to ask if they want to fix them up. This should be possible since you don't need admin permissions to read HKCR. The second method being adding a button in the settings which allows the user to manually run the process at their convenience.

To avoid the game ever running in UAC mode itself, it should invoke a second instance, probably after letting the user know what's going on, via Process.Start.

So the game will apply the registry changes after relaunching with admin, then spawn a new process with user level permissions when its done? This would work, but we should definitely make sure that the initial user-level process gets disposed of properly, probably via Environment.Exit(0); to avoid any unnecessary overhead.

it's best we outline exactly how it will work to avoid any wasted effort

Fair enough. I think that implementing it into the startup sequence and adding the logic there works. I've already more or less written the logic for adding/editing the keys already. At the moment its more about addressing the permissions and placing the code in the right place and having it all work together.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 15, 2024

and if they look out of place, prompt the user to ask if they want to fix them up

Precisely.

So the game will apply the registry changes after relaunching with admin, then spawn a new process with user level permissions when its done?

No, it would stay running and invoke a second instance which doesn't start the game but only sets associations.

I'd almost be inclined to copy the code from stable since we already know that it works quite reliably.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 15, 2024

@smallketchup82 in further discussion with the guy that makes the installer/updater we use, it seems like this may not require UAC these days, by using HKCU instead (HKEY_CURRENT_USER\SOFTWARE\Classes). Worth investigating initially.

@smallketchup82
Copy link
Contributor

@smallketchup82 in further discussion with the guy that makes the installer/updater we use, it seems like this may not require UAC these days, by using HKCU instead (HKEY_CURRENT_USER\SOFTWARE\Classes). Worth investigating initially.

I've looked into that, it might work but I think it would break backwards compatibility with stable. Most people will be using lazer alongside stable, so messing up registry keys for both is not a good idea lol. We should probably approach with caution if we decide to go that route. I think that if we plan to change how we set registry keys for file associations, it would be best to have the same implementation for both stable and lazer. If we can get rid of UAC on both, that's an added bonus. It would suck to have both versions fighting each other for file associations

I'll definitely need to read more into seeing if HKCU is a viable alternative though. When I checked before, I found that apparently it uses executable names instead of paths, but that doesn't make any sense to me, so I have to be wrong about that.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 15, 2024

Conflicting with stable can be fixed as an afterthought.

@smallketchup82
Copy link
Contributor

smallketchup82 commented Jan 15, 2024

Conflicting with stable can be fixed as an afterthought.

Fair enough, I guess migrating to using HKCU would be the play then. It would require rewriting the registry rules. From what I understand, stable creates 2 registry keys in HKCR for the url handler and the beatmap file handler. It then creates keys in HKCU to point the extensions .osz, .osk, .osu, .osr, and the rest, to the beatmap handler in HKCR.

Our approach would probably be to point the file extensions in HKCU to the lazer executable instead of the beatmap handler. After doing some reading on it, I think it'll work.

This would address the file associations but not the url handler, I'll need to see what we can do about that.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 15, 2024

Sounds good.

Note that we'll be upgrading the squirrel version soon which will mean that the main executable is in a stable place, but pointing to the stub executable should work fine for now.

@smallketchup82
Copy link
Contributor

I'm inclined to just have lazer point them to Assembly.GetEntryAssembly().Location and check to make sure that this is true every time on startup. This way lazer can update the file associations to the right path even if its installed in a custom location.

@smallketchup82
Copy link
Contributor

@peppy So I've gotten an initial implementation (rough draft) of what we discussed in my branch. It works pretty well. Doesn't need admin permissions, and can be run anytime and anywhere as a result.

There's just one issue I stumbled upon. The keys set by stable in HKCR seem to take priority over the user defined ones. During my testing, my windows PC would keep launching the files in stable until I deleted the respective registry key in HKCR. I might just be wrong and stable was overwriting my HKCU keys every time it was being launched (which I observed happening), and that I managed to refresh file explorer during a time where all of the keys were correctly taking priority in HKCU.

Either way, if the system level associations are taking priority over the HKCU values, this indicates a big problem since microsoft themselves say that the user level keys should be taking priority over the system level ones.

@caesay
Copy link

caesay commented Jan 15, 2024

HKCR is a a merged view of both HKEY_LOCAL_MACHINE\Software\Classes and HKEY_CURRENT_USER\Software\Classes. You probably should not write directly to HKEY_CLASSES_ROOT.

https://learn.microsoft.com/en-us/windows/win32/sysinfo/hkey-classes-root-key

To change the settings for the interactive user, store the changes under HKEY_CURRENT_USER\Software\Classes rather than HKEY_CLASSES_ROOT. To change the default settings [for the machine], store the changes under HKEY_LOCAL_MACHINE\Software\Classes. If you write keys to a key under HKEY_CLASSES_ROOT, the system stores the information under HKEY_LOCAL_MACHINE\Software\Classes. If you write values to a key under HKEY_CLASSES_ROOT, and the key already exists under HKEY_CURRENT_USER\Software\Classes, the system will store the information there instead of under HKEY_LOCAL_MACHINE\Software\Classes.

Per the documentation, writes to HKEY_CLASSES_ROOT can end up in either place, and will overwrite values in HKEY_CURRENT_USER if they exist.

@smallketchup82
Copy link
Contributor

smallketchup82 commented Jan 16, 2024

HKCR is a a merged view of both HKEY_LOCAL_MACHINE\Software\Classes and HKEY_CURRENT_USER\Software\Classes.

I'm aware of this, just find it weird how deleting the stable osu keys from HKCR fixes it and activates the user level rules when the user level (HKCU) rules are supposed to be the ones taking priority over HKCR & HKLM and should be active whether the keys are in HKCR or not. As explained in the docs

You probably should not write directly to HKEY_CLASSES_ROOT.

That's the goal. Writing to HKCR would require admin permissions which we want to stay away from.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 16, 2024

The keys set by stable in HKCR seem to take priority over the user defined ones

As I touched on before, this is fine. We can fix this as an afterthought. The migration procedure from stable to lazer still needs some work and I'd see this being part of whatever we do to make that procedure smoother.

@smallketchup82
Copy link
Contributor

smallketchup82 commented Jan 21, 2024

Okay so its been a week since I last touched on this, but given that it's priority 1, It would be best to get this stuff addressed and put out ASAP. My other changes are also in the midst of being reviewed so I suppose it would be wise to work on this in the meantime.

The keys set by stable in HKCR seem to take priority over the user defined ones

As I touched on before, this is fine. We can fix this as an afterthought. The migration procedure from stable to lazer still needs some work and I'd see this being part of whatever we do to make that procedure smoother.

Leaving this unaddressed and dismissing it as a "stable issue" effectively makes adding lazer's associations to HKCU useless, since they'll open with stable even if lazer's keys are set, as stable's keys in HKCR are prioritized. You say that migration from stable -> lazer needs to be refined but I'm unsure as to what you mean by this. Changing registry keys after the fact is going to be a hard thing to do, since HKCR requires admin permissions to edit. If you want to make stable associate in HKCU, that will require all existing players to either let the game run with admin permissions so that it can fix up the associations (remove the HKCR values) then relaunch, or reinstall the game completely.

Please tell me what you intend to do. Both to lazer and to stable. So that I don't waste effort on finding and adding a countermeasure to something that won't ever exist.

@smallketchup82
Copy link
Contributor

smallketchup82 commented Jan 21, 2024

As for what I've gotten done already, I've written the base code for associating files with Lazer. I need to still implement the url handler, also add names to the handlers so that they don't show up in file explorer as "osu! File" and instead as "osu! beatmap" or the such, and also make it run on first time setup without prompting the user. Given that those three, and also the issue that I'm discussing with peppy are resolved, then this should be more or less finished.

I'll probably create a draft PR in the meantime so that I can get some feedback on what I've gotten so far & track/organize my changes/todo more neatly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform/windows priority:1 Very important. Feels bad without fix. Affects the majority of users.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants