redhawk989 / eyetrackvr-installer Goto Github PK

The installer cannot continue while downloading,because of China's network environment?(I am Chinese)

Hi I'm literally only going to critique the installer a lot because I've been working on an installer for Amethyst for the past few months and seeing what this installer is like is just AAAAA. Anyway here's the big list of things that you should fix:

1. Embed native DLLs as an Embedded Resource. Create a temporary directory, and extract the resource to said directory. Here's how we handle extracting the native binary "openvr_api.dll" which we use to locate the SteamVR binaries, is handled in Amethyst Installer:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void LoadOpenVRAPI() {

    var ovrDll = Path.GetFullPath(Path.Combine(Constants.AmethystTempDirectory, "openvr_api.dll"));
    Util.ExtractResource("Binaries.openvr_api.dll", ovrDll);

    // Load the openvr_api.dll unmanaged library using P/Invoke :D
    var result = Kernel.LoadLibrary(ovrDll);
    if ( result == IntPtr.Zero ) {
        Logger.Fatal("Failed to load openvr_api.dll!");
        Logger.Warn("Falling back to openvrpaths.vrpath...");
        s_failedToInit = true;
    } else {
        Logger.Info("Successfully loaded openvr_api.dll!");
    }
    s_initialized = true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ExtractResource(string resourcePath, string filePath) {
    using ( var resource = Assembly.GetExecutingAssembly().GetManifestResourceStream($"amethyst_installer_gui.Resources.{resourcePath}") ) {
        using ( var file = new FileStream(filePath, FileMode.Create, FileAccess.Write) ) {
            resource.CopyTo(file);
        }
    }
}

[DllImport("kernel32", SetLastError = true, CharSet = CharSet.Unicode)]
public static extern IntPtr LoadLibrary(string lpFileName);

2. Why is the installer using .NET 6.0, and not .NET Framework? It's 150 MB, that's too large for an installer. Every Windows install since Windows 7 has .NET Framework pre-installed, take advantage of that (that means you don't need to include the entire runtime with the installer binary)! An installer should be tiny in file size and have 0 dependencies (it's an installer, the whole point is to setup everything on your system properly for whatever you're installing).

3. What's the design language you're going for? The installer has "fancy"-ish UI (more-on that in a bit) but is inconsistent in terms of design with the actual application. People like me who have OCD exist and this sort of inconsistency makes my blood boil.

4. What with the whitespace in the code? Clean it up!

5. Just wait for the file to download; This is bad practice.

6. Wrap the WebClient in a using statement so that it disposes when it goes out of scope. Then only use one WebClient for both of them.

7. Why are you using WebClient then HttpClient? HttpClient can make a GET request perfectly fine (see HttpClient.GetStringAsync() )

8. You can abstract opening a link to a method, you've got it 3 times already.

9. You should make an uninstaller too. Here's the documentation for the uninstaller registry keys from MSDN for reference. ALSO EXTREMELY IMPORTANT: make sure you don't format user's drives. PLEASE IMPLEMENT THIS IF YOU WRITE AN UNINSTALLER.

10. Also use IlMerge to merge .NET libraries together into a single binary. That way you can distribute a single exe file to your users, instead of a massive ZIP archive.

11. Use Checkbox.IsChecked instead of this. It's a literal property in the Checkbox class.

12. Nitpick: Use Path.Combine("some/path", "directory"); and Path.GetFullPath("some\path/with\random/backslashes");

13. Sanitise the folderPath variable. I could type in Among us into the folder path and your installer will explode. TLDR: Make sure that the folder the user typed exists.

14. The padding on the "Downloading..." button bugs me...

15. Consider making a successful installation more obvious, either by showing the user a dialogue box (see MessageBox.Show()) or something else that grabs the user's attention.