-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Cross-platform Stride.Launcher #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
base: master
Are you sure you want to change the base?
Conversation
7b72320 to
03f94a4
Compare
| /// Returns path of Launcher (we can't use Assembly.GetEntryAssembly().Location in .NET Core, especially with self-publish). | ||
| /// </summary> | ||
| /// <returns></returns> | ||
| internal static string? GetExecutablePath() => Environment.ProcessPath; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: Could this property not be used directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I just replaced code from the previous implementation to make it cross-platform without changing the overall structure. We could inline it manually.
On the other hand, the method name is a bit more explicit. It's likely inlined anyway by the compiler, so it's just a matter of preference.
| } | ||
|
|
||
| // Avalonia configuration, don't remove; also used by visual designer. | ||
| public static AppBuilder BuildAvaloniaApp() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: This method seems to be unused. It would be great to integrate with RunNewApp somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used by the Avalonia plugin. It's needed for the designer.
…e is already running
03f94a4 to
e96d938
Compare
|
@Jklawreszuk I have merged back master to this PR's branch. It's mostly working but it would be nice to also support (un-)installing the Stride packages on Linux even if we can't fully use it there yet. Did you want to contribute to it? The branch is on this repo, so feel free to push commits if you want. |
|
I am sorry, but it took some time to review all the files.Overall, it looks great! A quite accurate implementation of the original Launcher. However, I have a few comments:
Other than that I didnt notice nothing unusual 😅 |
|
@Kryptos-FR Once the changes have been merged, I will try to adapt Launcher to Linux if necessary. But I think that installing/uninstalling packages works correctly, no? |
|
I don't remember if it does work. I assume since it's restoring packages it should. I guess we'll know soon. |
PR Details
New Launcher using Avalonia instead of WPF. Still needs some love.
Related Issue
#1503
Types of changes
Checklist