Removes emojis from messages on Windows, adds 'SystemInfo' class to retain imutable information about the SO environment

[edassis]

Hi,

This is a patch originated from the desire to fix the messages logs with broken emojis when running on Windows, as it's console doesn't use UTF-8 encoding (good reference about cmd it here: https://stackoverflow.com/a/75788701/8903027, documented this link in the Log class for future reference).

To make it work, the Log.Output() class now checks if the system running is Windows, if it is, removes from the message all leading non-ASCII (emojis) chars before printing.

To do this, the Log class needs system information. The way I approached it, avoiding changing a bunch of methods/interfaces signatures, was to create a class that holds these informations, and that's how SystemInfo static class came about. The idea it's that the information inside it should be ready when the program starts (class initialization) and should not change during run-time. I thought of making it a record, but as it has a constructor, I keep it as a class (but maybe we can refactor it to something more elegant or that better fit with the code). Anyway, I think it's a new idea in the codebase that others systems could benefit of.

The abstraction SystemInformation was created, containing CPU and OS information that should not change during runtime. I tried to create it as a static class previously to avoid having to change signatures in multiple layers, but it showed very cumbersome to manage the SystemInfo lifecycle and to mock it during tests. So I scratched that and now, it's a normal class created at the main method that is being propagated to the others objects from there.

Another change was in the logs. Changed the Info() color to cyan to make it more visible, adjusted install command messages to make them more concise and dense:

• Each step has a log saying what is about to happen, defined as Info (cyan);
• Details intrinsic of the step it's printed normally (white). Some messages that could be considered details of the step were being printed as Info, but this diminishes the importance of the real important Info messages (as it tends to fall into normality for the human eyes). So, I tried to use more sparingly the Info type in the messages;
• At the end of the step, the confirmation message it's printed, defined as Success.

Most of these points are already there, just did I touch in consistency. If these changes in the log style are well welcomed we can look to extend it to all the CLI logs and not just the install command.

Windows preview

Main branch:

Patched:

Linux preview

Main branch:

Patched:

One point I'm having some difficult to understand it's how the progress text works for the Godot extraction process. During my tests, seems to me that the string message sent by Progress<double>((percent) => { ... } stays in the stream output, and clashes with the output of the next message. But I think I circumvented it by making log.Print("") to output a new line followed by log.ClearCurrentLine() to reset the cursor to the beginning of the line.

Another bug that I may be found: the extraction process not reaches 100%, despite the process having completed the text doesn't reach 100%:

Extracting text not reaches 100%

This PR delivers a fix in the Windows symlink creation routine. At the beginning, we were creating a symbolic link to point to Godot's binary, but to offer desktop shortcut support, we added the creation of hard links used by the desktop shortcut, and the symbolic link stayed. As having the hardlink covers the previous usage for the symbolic link, I removed the symbolic link creation on Windows and standardized that: on Windows, if it's not a directory link, it will be a hard link.

WIP:

• Code clean up;
• Code Tests;
• Test on Mac (help wanted, I don't have a Mac :/).

Let me know what you think.

Hope the holidays were good for you! 😎

[jolexxa]

Hey, thanks for tackling this one. I'm happy to test this on mac once you get this cleaned up and passing CI.

[edassis]

Ready for review.

[jolexxa]

@edassis Thank you for tackling all of this, it seems like a lot of really nice (but painful to implement) quality-of-life improvements. I'm going to try and get to testing this locally on my mac in the next few days — just been really busy.

[edassis]

Okay :), no problem at all. Let me know in case it needs some adjustments.

[jolexxa]

@edassis I've got a big ol' sticky note on my desk reminding me to get to reviews for this tonight / this weekend. That you for all the awesome contributions. I imagine there will be a few things we'll need to adjust, but nothing major.

[jolexxa]

This cleaned up really nicely. The only really refactor I'd like to see is putting SystemInfo in the execution context so you don't have to pass it in to create logs. Other than that and a few stragglers, we're looking really good here.

[jolexxa]

thank you 💜