Skip to content

[Ref/Feat] System Information #3027

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

Merged
merged 9 commits into from
Sep 13, 2023
Merged

[Ref/Feat] System Information #3027

merged 9 commits into from
Sep 13, 2023

Conversation

CommandMC
Copy link
Collaborator

This PR's main goal was to refactor the way we acquire system information. For this, several functions were split off from src/backend/utils.ts into their own files/folders, and a few new ones were added
Once that was done, I had the idea to use this information in a dedicated Frontend component, so now we also have a "System Information" tab in the "Settings" category of the sidebar, which allows for easy access of this information (particularly useful if it's needed for support requests)

I've tested this on:

  • Linux with a desktop PC & VM
  • Windows with a desktop PC & VM
  • Steam Deck
  • macOS

Some other highlights include:

  • We can now know for certain whether a device is a Steam Deck
  • Heroic Version codenames are now kept inside package.json for ease-of-access
  • A new hook, useAwaited, was introduced to easily work with functions returning promises in React. This is particularly useful for IPC

Tests for the various new functions would be neat, but the PR's already getting rather large


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@CommandMC CommandMC added the pr:ready-for-review Feature-complete, ready for the grind! :P label Sep 4, 2023
@CommandMC CommandMC requested a review from arielj September 4, 2023 01:12
@CommandMC CommandMC self-assigned this Sep 4, 2023
@flavioislima
Copy link
Member

Tested here on linux and works pretty nice, will try on macOS as well:
image

CPU: 20x 13th Gen Intel(R) Core(TM) i7-13700H
Memory: 33.34 GB (used: 5.91 GB)
GPUs:
  GPU 0:
    Name: NVIDIA Corporation AD106M [GeForce RTX 4070 Max-Q / Mobile]
    IDs: D=2820 V=10de SD=13c0 SV=1462
    Driver: nvidia,  GPU 1:
    Name: Intel Corporation Raptor Lake-P [Iris Xe Graphics]
    IDs: D=a7a0 V=8086 SD=13c0 SV=1462
    Driver: i915
OS: BigLinux based in Manjaro Linux 6.5.0-1-MANJARO (linux)

The current system is not a Steam Deck

Software Versions:
  Heroic: 2.9.1 Boa Hancock
  Legendary: 0.20.32 Dark Energy (hotfix #6)
  gogdl: 0.7.3
  Nile: 1.0.0 Jonathan Joestar

@flavioislima flavioislima requested review from a team, flavioislima, Nocccer and imLinguin and removed request for a team September 4, 2023 12:25
@flavioislima
Copy link
Member

Tested on macOS now:
image

I think that the memory is not very accurate but might be related on how mac reports it?
And it is also slower to get the info on mac than on Linux, just an observation.

Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great.
That's a pretty nice refactor and works pretty well ⚔️

@CommandMC
Copy link
Collaborator Author

Memory use is inaccurate on macOS and Linux (there's a FIXME about that in the function); Electron's reported value is the free amount of RAM, while we should be displaying the available amount (which doesn't include disk cache). We'll have to run the free command to get that value

The "CPU" card looks a little empty like this, should probably add a logo for Apple

@imLinguin
Copy link
Member

I think this memory inconsistency should be resolved, to avoid confusion among Deck and inexperienced Linux and Mac users. Seeing above 90% memory usage reported by Heroic may be misleading

Copy link
Member

@imLinguin imLinguin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition, looks great! MacOS codenames would be nice touch but we can postpone that to be added in the future if you feel like it

`getHericVersion` outside of utils.ts

The behavior of everything but `getHeroicVersion` stayed the same.
Heroic's own version codenames will now live inside `package.json` (right next
to the version number) for convenience
This also adds the `hasExecutable` shorthand
We've had variations of this in several smaller `util` files, let's go for
making one generic one instead
Versions of something like this (`callRunner` for example) have existed as
purpose-built functions, but implementing something new that intents to spawn
processes always needed a new "spawn this process with these arguments and give
me the output" function along with it. Time to change that
This is essentially the `systeminformation` module, but tailor-made for our use
case (no longer getting stuck, gathering more information, respecting
Flatpak paths)
'settings.systemInformation.systemSpecifications',
'System Specifications:'
)}
</h5>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe nitpicky here, but we have the h3 and h5 sounding redundant
image

Maybe we can just leave the h3? unless there's a reason to put that h5 there (maybe the idea is to put more content?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had clarity in mind with this (your CPU, memory, and GPUs are your specifications, while everything on the page in general is information), but I'd be fine with removing it as well

@CommandMC
Copy link
Collaborator Author

Other than implementing the review comments, I've added f7d042b now (since it's a common topic in support requests)

@CommandMC CommandMC requested a review from arielj September 10, 2023 16:29
@flavioislima flavioislima merged commit fa20192 into main Sep 13, 2023
@flavioislima flavioislima deleted the ref/sysinfo branch September 13, 2023 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants