Skip to content
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

Infra: Multi-platform support #36

Draft
wants to merge 54 commits into
base: main
Choose a base branch
from
Draft

Infra: Multi-platform support #36

wants to merge 54 commits into from

Conversation

NoobNotFound
Copy link
Member

@NoobNotFound NoobNotFound commented Jul 8, 2024

add multi platform support (wip)

Summary by Sourcery

Add multi-platform support by introducing platform-specific configurations and assets, refactor JSON serialization to use System.Text.Json, and enhance the build and CI processes with new scripts and workflows. Implement a new CoreX module for handling Minecraft mods and profiles, and update issue templates for better bug reporting and feature requests.

New Features:

  • Introduce multi-platform support by adding new platform-specific configurations and assets for MacCatalyst and other platforms.
  • Add a new CoreX module to support different Minecraft installation profiles and mods.
  • Implement a new ModrinthStore class for handling mod, plugin, resource pack, shader, and modpack downloads from the Modrinth API.

Enhancements:

  • Refactor JSON serialization from Newtonsoft.Json to System.Text.Json across multiple files for improved performance and consistency.
  • Update issue templates for bug reports and feature requests to include more detailed fields and platform-specific options.
  • Add a new FileDownloader class to handle file downloads with integrity checks using SHA-1 and SHA-512 hashes.

Build:

  • Add a PowerShell script to automate the installation of the Windows SDK ISO for build environments.
  • Introduce a global.json file to manage SDK versions and dependencies.

CI:

  • Create a new GitHub Actions workflow for continuous integration, including steps for dependency installation and project build.
  • Add a custom action to install platform-specific dependencies using Uno.Check.

Documentation:

  • Add a ReadMe.md file in the Emerald directory to guide users on getting started with the Uno Platform.

@NoobNotFound NoobNotFound requested a review from a team as a code owner July 8, 2024 13:22
@Lamparter

This comment was marked as outdated.

Copy link
Contributor

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Maybe you guys can use Directory.Build.props to remove duplicated properties among projects?

Emerald.App/Emerald.App/Emerald.App.csproj Outdated Show resolved Hide resolved
0x5bfa
0x5bfa previously approved these changes Jul 12, 2024
Copy link
Contributor

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Anyway, it looks ok!

This is one of the reasons that UWP is better 😭
I remember multi-platform support was so easy...

WinAppSdk is more desktop app like. I feel you pain as we’ve used it in FluentHub.

Lamparter and others added 5 commits July 12, 2024 21:32
Signed-off-by: Lamparter <notlamparter@outlook.com>
Signed-off-by: Lamparter <notlamparter@outlook.com>
Signed-off-by: 0x5BFA <62196528+0x5bfa@users.noreply.github.com>
@NoobNotFound
Copy link
Member Author

This is one of the reasons that UWP is better 😭 I remember multi-platform support was so easy...

yeah I agree to @0x5bfa this was UWP at the first (SD Launcher) but it was so difficult to even a run a single Minecraft version because accessing resources is limited. This originally created the option 'Run Minecraft As Admin' because it would solve it at some kind of security risk

@NoobNotFound
Copy link
Member Author

NoobNotFound commented Jul 13, 2024

btw I planned to rewrite the Emerald.Core to Emerald.CoreX with a well (i'll try my best) organized code pattern so it would be easy to maintain.
I was away from the dev for kind of a time so I don't even understand somethings I have written before lol.

And I'll mostly focused on bringing Emerald for MacOS but emerald for Linux will be also created since I hope to use single framework for all.
Emerald for Windows works with no problem at all so I think it'll be a mess to replace Core with CoreX in Emerald.WinUI

BTW what do you guys think should I use for emerald cross platform?
would it be

  • MAUI
  • MAUI Hybrid (Blazor based MAUI app)
  • Blazor with Something like PWA or electron to create a desktop like (idk even this is possible at all lol)
  • Uno Platform
  • Or something else (Because I think all of these are not just perfect as native WinUI and apple UI (idk whatever they call it)

@RiversideValley/emerald what do you think?

@Lamparter
Copy link
Member

@RiversideValley/emerald what do you think?

Okay.....
So first things first. In terms of cross-platform, Uno seems like the best way to go as it is the most mature multi platform runtime engine. Blazor/Electron/ASP.NET apps are literally terrible and I wouldn't recommend them in a billion years as they are slow and buggy and the codebase is just messy - desktop apps shouldn't programmed in the same way as websites in my opinion. Plus, ASP.Net Core/Blazor/Electron apps are limited to most OS permission scopes as a website and may be able to open it but wouldn't function as a launcher in the same way that the Minecraft Launcher/Natsurainko.FluentLauncher/Lunar/Badlion/(whatever launchers are being used nowawadays) in terms of being actually able to manage the game on demand - you might as well be developing Emerald as a website that just launches Minecraft via protocols which betrays the true meaning behind what a Minecraft Launcher really is.
Emerald for macOS, on the other hand, will be difficult. I'm not sure whether .NET desktop apps designed and managed by Uno are even supported on macOS.
Then again, this leads me to question, why do we want Emerald to work as a cross-platform app? It's fine to make it universally available across different Windows versions (AMD64, Arm, Arm64, WinRT, etc.) but why do we really want it to function on different platforms? Minecraft on macOS is the only comparable version of the game to that of Windows and I am to suppose that we will have very little userbase who are interested in a Cupertino UI (what Apple's design system is called) Minecraft Launcher that lacks the features or core contributors of other major competitors or even the default Launcher itself? On top of that, Minecraft for Linux is incredibly underdeveloped and resembles Classic Minecraft in its entirety - not a proper game as Minecraft on Windows and macOS appears to be.
Another thing - codebase unifier platforms like Uno and MAUI generally lean towards having a single codebase as that is literally the meaning of the word Uno - it makes no sense to have a separate project for each version as all the apps being able to be built from a singular codebase is the very nature of Uno and MAUI in the first place.
All in all, this is merely speculative but it really does make me question what Emerald really is.

@NoobNotFound
Copy link
Member Author

Okay..... So first things first. In terms of cross-platform, Uno seems like the best way to go as it is the most mature multi platform runtime engine. Blazor/Electron/ASP.NET apps are literally terrible and I wouldn't recommend them in a billion years as they are slow and buggy and the codebase is just messy - desktop apps shouldn't programmed in the same way as websites in my opinion.

Yeah I do also prefer Uno over the others, Just listed down the possible ways of doing this. And also, I have never tried either Uno or Blazor with electron(ized) before. So I may need time to get these thing well.

Plus, ASP.Net Core/Blazor/Electron apps are limited to most OS permission scopes as a website and may be able to open it but wouldn't function as a launcher in the same way that the Minecraft Launcher/Natsurainko.FluentLauncher/Lunar/Badlion/(whatever launchers are being used nowawadays) in terms of being actually able to manage the game on demand - you might as well be developing Emerald as a website that just launches Minecraft via protocols which betrays the true meaning behind what a Minecraft Launcher really is.

Just to mention, ig some of the popular (legal or cracked) minecraft launchers like TLauncher, Lunarclient uses HTML UI with Java (I'm not sure since I haven't observed any Java based app before, but I think it works like that)

I don't like Electron\Blazor UI based apps because of the performance. The only thing I like about it is we can craft whatever unique UI we want with HTML and CSS (although I'm not a web designer lol)

Emerald for macOS, on the other hand, will be difficult. I'm not sure whether .NET desktop apps designed and managed by Uno are even supported on macOS.

There is some built with Uno for macOS, like Devtoys, Ryujinx (An amazing project I found when searching for emulators, not so sure whether it is poweren by Uno)

Then again, this leads me to question, why do we want Emerald to work as a cross-platform app? It's fine to make it universally available across different Windows versions (AMD64, Arm, Arm64, WinRT, etc.) but why do we really want it to function on different platforms?

well Minecraft JAVA Edition is cross-platform, so why not emerald?
Actually I like to bring emerald for macOS since I have a mac tbh.

Minecraft on macOS is the only comparable version of the game to that of Windows and I am to suppose that we will have very little userbase who are interested in a Cupertino UI (what Apple's design system is called)

Yeah I agree, but I have experienced that annoying user experience when I try to install 3rd party MC launcher on my new mac.
I had to manually install Java runtime, finding the correct runtime could be challenging not to me but to other normal non-dev peoples like my younger brother who only knows to open emerald and launch minecraft.
Since CMLLib automatically installs required java rumtime which is very easy for normal users.

Minecraft Launcher that lacks the features or core contributors of other major competitors or even the default Launcher itself? On top of that, Minecraft for Linux is incredibly underdeveloped and resembles Classic Minecraft in its entirety - not a proper game as Minecraft on Windows and macOS appears to be.

I had only a linux based device before (Which actually came with windows but had to install a linux because it was too old for Windows). I played Minecraft on that device using Tlauncher, I was happy with it. (even though the performence was not so great)
I think Emerald would easily work if it was cross platform thanks to the well-written CMLLib.

Another thing - codebase unifier platforms like Uno and MAUI generally lean towards having a single codebase as that is literally the meaning of the word Uno - it makes no sense to have a separate project for each version as all the apps being able to be built from a singular codebase is the very nature of Uno and MAUI in the first place. All in all, this is merely speculative but it really does make me question what Emerald really is.

yeah, so do I 😀. Emerald is just a UI for some "custom minecraft launcher" Libraires. made for easy of use, easily download mods using Modrinth API and install and run moded MC version just like TLauncher.

btw what would be the final decision? what about. MAUI? actually I really don't love any of these, I would rather build a native mac app if I knew SwiftUI

@NoobNotFound
Copy link
Member Author

wow Ive written a whole essay lol

@Lamparter

This comment was marked as off-topic.

@Lamparter

This comment was marked as off-topic.

@Lamparter

This comment was marked as outdated.

@Lamparter
Copy link
Member

Also, I just remembered, MAUI doesn't support Linux so it wouldn't work that way anyway

@Lamparter Lamparter marked this pull request as draft July 14, 2024 17:46
@Lamparter Lamparter changed the title Multi platform support 🏗️ Multi-platform support Jul 14, 2024
@NoobNotFound
Copy link
Member Author

Also, I just remembered, MAUI doesn't support Linux so it wouldn't work that way anyway

so, a one solution is gone lol

@NoobNotFound

This comment was marked as off-topic.

@Lamparter
Copy link
Member

Yeah it's a shame since MAUI is very much supported by Microsoft and it would be very nice to get to use it while its new because I imagine that in the near future Microsoft will standardise it as the core universal multi-platform framework for .NET

Copy link
Member

@Lamparter Lamparter left a comment

Choose a reason for hiding this comment

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

I mean, yeah the CoreX project does establish a difference between winui core library and multi platform library but I don't really see it's point

Emerald.CoreX/Class1.cs Outdated Show resolved Hide resolved
Emerald.CoreX/Emerald.CoreX.csproj Show resolved Hide resolved
Co-authored-by: codefactor-io <support@codefactor.io>
Copy link
Member Author

Choose a reason for hiding this comment

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

currently this does not support continuing from interrupted download or pausing, but it should be implemented soon.

@Lamparter
Copy link
Member

I read online that apparently if you do this command:
@sourcery-ai review

@Lamparter

This comment was marked as off-topic.

Copy link

sourcery-ai bot commented Sep 10, 2024

Reviewer's Guide by Sourcery

This pull request implements multi-platform support for the Emerald project. The changes include updates to issue templates, modifications to existing code for cross-platform compatibility, and the addition of new files and directories to support multiple platforms. The most significant changes involve switching from Newtonsoft.Json to System.Text.Json, updating the project structure to support Uno Platform, and adding new components for different platforms.

File-Level Changes

Change Details Files
Switched from Newtonsoft.Json to System.Text.Json for JSON serialization and deserialization
  • Replaced JsonConvert.SerializeObject with JsonSerializer.Serialize
  • Replaced JsonConvert.DeserializeObject with JsonSerializer.Deserialize
  • Updated method signatures and parameters to accommodate the new JSON serialization approach
Emerald.App/Emerald.App/Helpers/Settings/SettingsSystem.cs
Emerald.Core/Store/Labrinth.cs
Emerald.App/Emerald.App/Helpers/Settings/JSON.cs
Emerald.Core/News/NewsHelper.cs
Updated project structure to support Uno Platform
  • Added new platform-specific files and directories
  • Created new Emerald.CoreX project for cross-platform core functionality
  • Added Uno Platform configuration files and dependencies
Emerald/App.xaml.cs
Emerald/Platforms/MacCatalyst/Media.xcassets/LaunchImages.launchimage/Contents.json
Emerald/Assets/SharedAssets.md
Emerald/Properties/launchSettings.json
Emerald/Platforms/Desktop/Program.cs
Emerald/Platforms/MacCatalyst/Main.maccatalyst.cs
Emerald/MainPage.xaml.cs
global.json
Emerald/GlobalUsings.cs
Emerald/ReadMe.md
Improved issue templates and GitHub workflows
  • Updated bug report template with more detailed fields
  • Added new feature request and spec templates
  • Created new GitHub Actions workflow for CI
  • Added dependabot configuration
.github/ISSUE_TEMPLATE/bug_report.yml
.github/ISSUE_TEMPLATE/feature_request.yml
.github/ISSUE_TEMPLATE/sample/bug_report.yml
.github/ISSUE_TEMPLATE/sample/spec.yml
.github/ISSUE_TEMPLATE/sample/feature_request.yml
.github/ISSUE_TEMPLATE/sample/config.yml
.github/workflows/ci.yml
.github/dependabot.yml
Implemented new store functionality for Modrinth
  • Created new ModrinthStore class with methods for searching, fetching, and downloading items
  • Implemented FileDownloader class for handling file downloads
  • Added support for different types of Modrinth stores (mods, plugins, resource packs, shaders, modpacks)
Emerald.CoreX/Store/Modrinth/ModrinthStore.cs
Emerald.CoreX/Store/Modrinth/JSON.cs
Emerald.CoreX/Store/Modrinth/Stores.cs
Emerald.CoreX/Helpers/FileDownloader.cs
Emerald.CoreX/Store/Modrinth/IModrinthStore.cs
Emerald.CoreX/Store/Modrinth/SearchSortOptions.cs
Added support for different Minecraft versions and mod loaders
  • Created MCVersion model to represent different Minecraft versions
  • Implemented interfaces and classes for mod loader installers (Fabric, Forge, LiteLoader, Optifine)
  • Added MCVersionType enum to distinguish between different version types
Emerald.CoreX/Models/MCVersion.cs
Emerald.CoreX/Enums/MCVersionType.cs
Emerald.CoreX/Installers/Fabric.cs
Emerald.CoreX/Installers/Forge.cs
Emerald.CoreX/Installers/IModLoaderInstaller.cs
Emerald.CoreX/Installers/LiteLoader.cs
Emerald.CoreX/Installers/Optifine.cs

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @NoobNotFound - I've reviewed your changes and found some issues that need to be addressed.

This review was edited by @Lamparter as it contained some very "wrong" information.

Overall Comments:

  • Consider breaking large changes like this into smaller, more focused pull requests in the future. This would make the changes easier to review and less risky to merge.
  • The new code, especially in complex methods like those in FileDownloader, could benefit from more detailed comments explaining the logic.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🔴 Security: 2 blocking issues
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟡 Documentation: 1 issue found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Emerald/Assets/SharedAssets.md Show resolved Hide resolved
using CmlLib.Core;
namespace Emerald.CoreX.Store.Modrinth;

public class ModStore : ModrinthStore
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider consolidating multiple store classes into a single class with an enum for store types.

The current implementation with multiple store classes (ModStore, PluginStore, etc.) introduces unnecessary complexity and violates the DRY principle. Consider simplifying this structure by using a single store class with an enum to specify the type. This approach would reduce code duplication and improve maintainability while preserving the flexibility to handle different content types.

Here's a suggested refactoring:

public enum StoreType
{
    Mod,
    Plugin,
    ResourcePack,
    Shader,
    Modpack
}

public class ModrinthStore
{
    private readonly StoreType _storeType;

    public ModrinthStore(MinecraftPath path, ILogger logger, StoreType storeType) : base(path, logger)
    {
        _storeType = storeType;
    }

    public ModrinthStore(ILogger logger, StoreType storeType) 
        : this(new MinecraftPath(MinecraftPath.GetOSDefaultPath()), logger, storeType)
    {
    }

    // Implement GetItemAsync and GetVersionsAsync as before

    public override async Task DownloadItemAsync(ItemFile file, IProgress<double>? progress = null, CancellationToken cancellationToken = default)
    {
        string projectType = _storeType switch
        {
            StoreType.Mod => "mods",
            StoreType.Plugin => "mods",
            StoreType.ResourcePack => "resourcepacks",
            StoreType.Shader => "shaderpacks",
            StoreType.Modpack => "modpacks",
            _ => throw new ArgumentOutOfRangeException(nameof(_storeType))
        };

        await base.DownloadItemAsync(file, projectType, progress, cancellationToken);
    }
}

This approach consolidates the functionality into a single class, eliminating the need for multiple nearly identical classes. It uses an enum to differentiate between store types, which is more maintainable and allows for easy addition of new types in the future. The DownloadItemAsync method uses a switch expression to determine the correct project type based on the store type.

This refactoring significantly reduces code duplication while maintaining the ability to handle different content types. It's more flexible and easier to maintain in the long run.

Copy link
Member Author

Choose a reason for hiding this comment

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

wow, is this free? also thanks for pointing out Ill take a look.

Copy link

Choose a reason for hiding this comment

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

Yes, the feedback and suggestions provided by Sourcery are indeed free! I'm glad you found the feedback helpful. If you have any questions or need further clarification on how to implement the suggested refactoring, feel free to ask. We're here to help!

.gitignore Show resolved Hide resolved
@Lamparter
Copy link
Member

Lamparter commented Sep 13, 2024

Maybe you guys can use Directory.Build.props to remove duplicated properties among projects?

We should do this 👀
We already did Directory.Packages.props in #42

@Lamparter Lamparter changed the title 🏗️ Multi-platform support Infra: Multi-platform support Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗️ In Progress/Review
Development

Successfully merging this pull request may close these issues.

Bug: Failed to initialise Minecraft path ``
4 participants