search

FlashpointProject / launcher

Launcher for Flashpoint Archive
https://flashpointarchive.org
Other
941 stars 54 forks source link

[BUG] Diagnostic info shows wrong OS name and architecture #406

Open n0samu opened 9 months ago

n0samu commented 9 months ago

Describe the bug On my PC (Windows 10 Pro 64-bit), the launcher's diagnostic info (Logs => Copy Diagnostics) says:

Operating System: Windows 10 Enterprise
Architecture:     ia32

To Reproduce Steps to reproduce the behavior:

  1. Click the Logs tab of the launcher
  2. Click Copy Diagnostics
  3. Paste and observe the values

Expected behavior Operating system and architecture should match what is shown in System Information and elsewhere.

Desktop (please complete the following information):

oblivioncth commented 8 months ago

Relevant source: https://github.com/FlashpointProject/launcher/blob/84b44cfe10a543dbdcca1b7ad8c565e00613d885/src/back/responses.ts#L1667

So there's two things going on here, but oddly enough the root issue seems to be a Windows quirk.

The launcher gets this info via Node.JS' os module, specifically os.version() and os.arch().

First, os.arch() specifically returns the architecture that the binary was compiled for, not the current host system. Since the launcher is a 32-bit application this will always show ia32. proccess.arch is an alternative to this, but I think it might suffer from the same limiation.

For os.version() things get weirder. The underlying C library that Node uses gets the name from Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProductName, which on my system is "Windows 10 Pro"; however, it seems that this library doesn't specifically request access to the 64-bit registry via KEY_WOW64_64KEY and just uses the default mask. Since the application is 32-bit, the checked value is redirected to Computer\HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\Microsoft\Windows NT\CurrentVersion\ProductName and... for whatever bizarre reason, on my system the value of that key is "Windows 10 Enterprise".

Seems like Microsoft goofed.

I suppose the only around both of these is to try to find a module that better handles querying information of the host operating system (though introducing another dependency just for this sure feels shitty), or pull these values manually with os-dependent code :/

EDIT:

Seems the architecture aspect of it has been addressed before. Easy on Windows, complicated everywhere else https://github.com/nodejs/node/issues/17036

oblivioncth commented 8 months ago

Fixed the edition name issue upstream, but of course it will be some time until the update is integrated into a Node release and then that release is integrated into an Electron release, and finally the Launcher is updated to use that XD.

https://github.com/libuv/libuv/pull/4191#pullrequestreview-1707452007