-
Notifications
You must be signed in to change notification settings - Fork 17
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
Refactor/win_elevation : Ask for elevation if not elevated #53
Refactor/win_elevation : Ask for elevation if not elevated #53
Conversation
Had a chance to run this on Windows 10 Pro with and without UAC -- Without UAC, everything just worked, no administrator prompts. With UAC turned all the way up, it was able to install Godot, but hung indefinitely when trying to update the environment variable. I was running this on my administrator account. I'm not sure what needs to be done, but it doesn't seem as robust as the one that spammed the elevation prompts. I'm open to ideas. |
I've just tested it again on my machine and I don't have the same problem with updating environment variables. I don't know why it doesn't work for you with the code from my branch... Did you have the administrator prompt with UAC turned all the way up ? When I tried for the first time the code from chickensoft-games:refactor/win-elevation, I had a similar problem (without an admin prompt) because the content of "CreateInstance" was not completely defined. But I didn't think this behaviour would be encountered with the version of the code in AdrasteonDev:refactor/win-elevation. |
For your information, while doing other tests, I also noticed a bug with my code that I hadn't seen before for non-administrator users. This does not affect other administrator users. When elevated, I've seen that UacHelper allows to check whether a user is an administrator or not. Maybe I'll see what happens if I use UacHelper again, re-executing the command as an authorised user only if the user is an administrator. This would at least avoid "spamming" the admin prompt for the case of admin users (which should cover most Godot users outside a professional context). I will probably add a commit for that in this branch. By the way, the bug also affect GodotEnv 1.6.0 for non-administrator users if the command is executed in a Windows shell that has been run as administrator (I don't know if it can be fixed). Nota bene: No problem for me if at some point you don't want to keep the changes from PR #51 and #53, and want to stick with the current behaviour. In that case, just remember to add a commit to undo the changes from this commit. |
I've just pushed the commit |
@AdrasteonDev Thank you for the thorough follow-up. I was able to re-test (using UAC all the way up and disabled on my admin account, same as before) and whatever you've done now works. With UAC on, I get a single prompt. With it off, everything works as expected. I don't mind adding these changes, so I'll merge this into my PR and we'll get that in. I do suspect that handling administrator-ness on Windows might make things more complicated and there's probably more edge cases, but I'm not terribly worried about it. |
ac9d465
into
chickensoft-games:refactor/win-elevation
No description provided.