Compatibility rework #59

rankynbass · 2023-06-24T22:48:50Z

This replaces the Dxvk Settings rework - #17.

Here's the compatibilitytools rework I mentioned a few weeks ago. This moves most of the bits that will change over to the XL.Core side, such as specific versions of wine or dxvk, where they will be stored, and what environment variables need to be set for each one.

I also moved the MacVideoFix over to XL.Core. I figure individual implementations of fixes can be done on the XL.Core side, and then passed to GameFixApply object.

Depends on goatcorp/FFXIVQuickLauncher#1348

And here's a link to a flatpak build if you want to test:
https://github.com/rankynbass/XIVLauncher.Core/releases/edit/compatrework-v1

Install with flatpak install xivlauncher-rework.flatpak, run with flatpak run dev.goats.testing.xivlauncher.

Blooym

Happy with the changes on the XLCore side to go forward once the submodule changes are in.

src/XIVLauncher.Core/WineManager.cs

src/XIVLauncher.Core/GameFixes/MacVideoFix.cs

Blooym

Overall happy with the changes to detect distribution, just have some small questions/thoughts about it

Blooym · 2023-06-25T09:00:57Z

src/XIVLauncher.Core/Program.cs

@@ -44,7 +44,9 @@ class Program

    private static readonly Vector3 clearColor = new(0.1f, 0.1f, 0.1f);
    private static bool showImGuiDemoWindow = true;
-
+    public static string Distro { get; private set; }


Might be an idea to make this an enum so we're more aware of the valid values for it, could then just assign an attribute of its url value so we cna download the wine version from it using that attribute name.

src/XIVLauncher.Core/Program.cs

marzent · 2023-06-25T09:13:39Z

Honestly not sure about moving game fixes to xlcore wholesale, maybe adding a parameter to it for the currently hosted version by SE would be cleaner, ultimately something like $"https://mac-dl.ffxiv.com/cw/finalfantasyxiv-{currentSeMacWrapperVersion}.zip".

Then it is still possible to add in dynamic version information fetching on the xlcore side later.

Blooym · 2023-06-25T09:16:22Z

Honestly not sure about moving game fixes to xlcore wholesale, maybe adding a parameter to it for the currently hosted version by SE would be cleaner, ultimately something like $"https://mac-dl.ffxiv.com/cw/finalfantasyxiv-{currentSeMacWrapperVersion}.zip".

Then it still possible to add in dynamic version information fetching on the xlcore side later.

The way gamefixes is setup here doesn't seem like too much of a problem to me since it allows passing it in from either side & it makes it easier to adjust fixes that aren't going to apply to the main repo anyway

marzent · 2023-06-25T09:34:04Z

it makes it easier to adjust fixes

I agree with that and that was probably the main motivation behind this change...

However xlcore is mostly a frontend for XIVLauncher.Common and this change muddies the waters a bit (there is for example a special exception in the game repair logic for these video files, you could add validation there in the future as well, but this is gonna become difficult to maintain by splitting parts of the codebase for convenience and not architecture)

Ultimately this is a similar argument to #10

Blooym · 2023-06-25T09:37:51Z

However xlcore is mostly a frontend for XIVLauncher.Common and this change muddies the waters a bit (there is for example a special exception in the game repair logic for these video files, you could add validation there in the future as well, but this is gonna become difficult to maintain by splitting parts of the codebase for convenience and not architecture)

I'm totally happy to keep things in XIVLauncher.Common but it would be nice to have an easier path to getting changes made in XIVLauncher.Common.Unix as right now any functional change we want made has to go through it.

marzent · 2023-06-25T09:56:43Z

Yeah XIVLauncher.Common has become a bit of a bottleneck for functional changes...

Having said that (slightly off-topic again) I think the correct video URL can be extracted from SEs sparkle feed, so this won't really need any adjustments in the future.

rankynbass · 2023-06-25T14:46:55Z

So should I revert this? We could also have GameApplyFixes to either accept an array/list as a contstructor arg (as I already did), or add a function to add fixes one at a time. Then you could just do something like

var gameFixApply = new GameFixApply();
gameFixApply.Add(new MacVideoFix(...));

marzent · 2023-06-25T16:49:55Z

Personally I am not a huge fan of adding game fixes from the xlcore side (as in xlcore holds their implementation), but that is a matter of taste.
Haven't had the time to look over everything and test in detail but it looks good on first glance (and thanks for keeping the wine and dxvk runners in XIVLauncher.Common).
Can you maybe rebase both this and goatcorp/FFXIVQuickLauncher#1348 (and drop 8fd716d)

This reverts commit 8fd716d.

rankynbass · 2023-06-25T21:22:27Z

Okay, reverted the video fix move.

marzent · 2023-06-27T05:37:31Z

src/XIVLauncher.Core/Distro.cs

+
+namespace XIVLauncher.Core;
+
+public enum WinePackage


Feels a bit weird having the wine package enum inside the Distro file..
This seems to be completely unused anyways

It's actually used in the WineManager to determine which wine package to download. I've renamed to DistroPackage now.

marzent · 2023-06-27T05:45:11Z

src/XIVLauncher.Core/Distro.cs

+
+    public static bool IsFlatpak { get; private set; }
+
+    public static void GetInfo()


This function is called GetInfo() and returns nothing, which is not very intuitive to use, especially with the static properties next to it.

All of this also only makes sense on a target system that needs wine (and even then only on Linux).

Each csproj has WIN32, OSX and LINUX constants defined, which could be used here.

Also not sure if this needs to be a top-level file here at all, there is also https://github.com/goatcorp/FFXIVQuickLauncher/blob/410cc9ed0624c37879b747ad69ba6e61c18c6f0f/src/XIVLauncher.Common/Platform.cs#L7

marzent · 2023-06-27T05:49:08Z

src/XIVLauncher.Core/DxvkManager.cs

+                throw new ArgumentOutOfRangeException();
+        }
+
+        var settings = new DxvkRunner(folder, url, Program.storage.Root.FullName, env, isDxvk);


Not sure if I understand the concept of a DxvkRunner that doesn't run DXVK, or even the concept of runner at all, since it is more of a install/setup step before launching the game.
Also if the DxvkRunner isn't holding DXVK configuration it is setting up WineD3D and/or MangoHUD.

marzent · 2023-06-27T05:51:47Z

src/XIVLauncher.Core/Program.cs

@@ -75,6 +75,7 @@ private static void SetupLogging(string[] args)

        Log.Information("========================================================");
        Log.Information("Starting a session(v{Version} - {Hash})", AppUtil.GetAssemblyVersion(), AppUtil.GetGitHash());
+        Log.Information("Running on {DistroName}, wine package set to {DistroPackage}", Distro.Name, Distro.Package.ToString());   


Again this only makes sense on Linux, the XIVLauncher.Common Platform enum is probably a better fit here.

marzent · 2023-06-27T05:59:16Z

src/XIVLauncher.Core/Program.cs

+        var wine = WineManager.Initialize();
+        var dxvk = DxvkManager.Initialize();
+        var wineoverride = "msquic=,mscoree=n,b;";
+        var wineenv = new Dictionary<string, string>();


Wine Environment variables are set a bit all over the place, some here, some in the WineManager and some inside CompatibilityTools.
Would be nice setting everything that must be set, be set in CompatibilityTools, but still allow passing in extra env vars (but ideally only from 1 place)

Fixed. Now it's all in CompatibilityTools

marzent · 2023-06-27T06:02:32Z

src/XIVLauncher.Core/Components/MainPage/MainPage.cs

+                        Program.CompatibilityTools.AddRegistryKey("HKEY_CURRENT_USER\\Software\\Wine\\Direct3D", "renderer", "gl");
+                }
+
+                Program.CompatibilityTools.RunInPrefix($"winecfg /v {winver}");


marzent · 2023-06-27T06:03:38Z

src/XIVLauncher.Core/Components/SettingsPage/Tabs/SettingsTabDebug.cs

@@ -12,7 +12,7 @@ public override void Draw()
    {
        ImGui.TextUnformatted("Generic Information");
        ImGui.Separator();
-        ImGui.TextUnformatted($"Operating System: {Environment.OSVersion}");
+        ImGui.TextUnformatted($"Operating System: {Distro.Name} - {Environment.OSVersion}");


This is should ideally be a XIVLauncher.Common Platform

I might go through and fix all of this at some point; do the full initialization of OS info in the Distro object (maybe rename it), and then change all the Environment.OSVersion.Platform checks to refer to the XIVLauncher.Common Platform value set in said Distro object. But I think there's enough going on atm. For now, I've added in some checks to make sure the Linux-specific stuff only shows up if it's actually on Linux.

marzent · 2023-06-27T06:06:11Z

src/XIVLauncher.Core/Components/SettingsPage/Tabs/SettingsTabDxvk.cs

+                    return null;
+                },
+            },
+            new SettingsEntry<string>("MangoHud Custom Path", "Set a custom path for MangoHud config file.", () => Program.Config.DxvkMangoCustom, s => Program.Config.DxvkMangoCustom = s)


This is a Linux-only config option, and even then only useful when MangoHUD is actually installed

For the moment, there's a function that looks for specific paths where Linux installs MangoHud, and it won't let you save your selection if it doesn't find it (it will show an error). I think the whole DXVK tab is really linux only. Only the Disabled/WineD3D setting would work on MacOS atm (if that), and only if you manually provided a wine release. Maybe there's DXVK for freebsd? Not sure

DXVK is a PE only binary that translates Windows dx calls into Windows Vulkan calls (simplified), so it will run on any platform that supports the necessary Windows Vulkan feature level which is Wine on Linux, FreeBSD, macOS (if the native unix implementation has all needed extensions) and even on Windows itself (where it is sometimes used to provide better frame pacing, or just in general better performance that the native Windows dx driver)

marzent · 2023-06-27T06:09:08Z

src/XIVLauncher.Core/DxvkManager.cs

+        var dxvkfolder = Path.Combine(rootfolder, "compatibilitytool", "dxvk");
+        var async = (Program.Config.DxvkAsyncEnabled ?? true) ? "1" : "0";
+        var framerate = Program.Config.DxvkFrameRate ?? 0;
+        var env = new Dictionary<string, string>


This is a bit of a design question (similar to the wine env vars), should these env vars be set here or inside the "runner", which gets passed a log file path and a dxvk config file

marzent · 2023-06-27T06:10:20Z

src/XIVLauncher.Core/DxvkManager.cs

+                env.Add("DXVK_HUD","0");
+                env.Add("MANGOHUD","1");
+
+                if (mangoHudConfig is null)


There is a lot of MagoHUD handling going on here, maybe better to factor that out into its own class

Moved the hud stuff to its own class. There's also now a function that checks for mangohud being installed, and will set hudtype to none if somehow the user ends up manually setting mangohud in the .ini file without having it installed. DxvkSettingsTab already will not let you save settings in that case.

* Changed names of classes to WineSettings and DxvkSettings * Removed a bunch of unused variables * Moved WINEDLLOVERRIDES * Update submodule

* Also split out Hud settings from Dxvk * Renamed some vars and funcs for better clarity * Removed some duplicate functions * Hopefully made a bit more platform independant.

* Added button for explorer with WineD3D * Changed SetWin7 to two buttons to set windows type quickly

rankynbass · 2023-06-29T03:34:13Z

So, in no particular order:

WineD3D vulkan was removed. Adding a reg key can be slow, and this option is quite buggy with wine-xiv.
Cleaned up the Distro stuff to make it more platform independent.
Cleaned up some variable names and function names to be less confusing (hopefully). Deleted some old leftover functions.
Moved all the WINEDLLOVERRIDES stuff to XIVLauncher.Common.Unix
Moved the WineManger, DxvkManager to XIVLauncher.Core.UnixCompatibility, since technically this can be compiled for Windows. Also split out the Hud stuff to a separate class just to make it a little easier to manage the code.

* Make Hud options a bit more clear * Add warning for MangoHud+ReShade+Dalamud

rankynbass · 2023-07-26T15:11:17Z

Closing and replacing with #64

rankynbass added 11 commits June 17, 2023 01:01

First successful build and launch.

c6e523a

Reposition submodule

903ee25

Merge branch 'main' into compatibility-rework

8363495

Minor fixes

9b05879

Update submodule

e687d03

Merge branch 'main' into compatibility-rework

3878e84

Move more login into runners

3de5ce9

More work done

3a7aca8

Move WineRunner, DxvkRunner to FFQL repo

c522e0f

Fix gamemode, update submodule

272f4d1

Move MacVideoFix to xlcore

8fd716d

Blooym previously approved these changes Jun 25, 2023

View reviewed changes

src/XIVLauncher.Core/WineManager.cs Outdated Show resolved Hide resolved

src/XIVLauncher.Core/GameFixes/MacVideoFix.cs Outdated Show resolved Hide resolved

Replace directives with GetDistro function

380e62a

rankynbass dismissed Blooym’s stale review via 380e62a June 25, 2023 02:03

rankynbass added 3 commits June 24, 2023 22:07

Move distro function to Program.cs, add flatpak detection

8314140

Use Dictionary instead of iterating array

2142c7d

Fix error by switching to .ContainsKey()

0637558

Blooym reviewed Jun 25, 2023

View reviewed changes

rankynbass added 3 commits June 25, 2023 12:46

Move distro detection to own class, use enum

74b4912

Forgot to save a couple files

5d75345

Revert "Move MacVideoFix to xlcore"

94f34c4

This reverts commit 8fd716d.

Merge branch 'main' into compatibility-rework

9067d4e

rankynbass added 2 commits June 25, 2023 15:50

Update submodule so it will build

ad51095

Fix slightly wrong WINEDLLOVERRIDE.

0098371

marzent requested changes Jun 27, 2023

View reviewed changes

rankynbass added 8 commits June 27, 2023 21:43

Changes to match with main XL repo. More to come.

6fe1b6b

* Changed names of classes to WineSettings and DxvkSettings * Removed a bunch of unused variables * Moved WINEDLLOVERRIDES * Update submodule

Remove WineD3D Vulkan

7afb7a2

Move Wine, Dxvk, Distro to UnixCompatibility

f7162a4

* Also split out Hud settings from Dxvk * Renamed some vars and funcs for better clarity * Removed some duplicate functions * Hopefully made a bit more platform independant.

Cleaned up some Distro-related stuff.

64a3a62

Update submodule

ea617f2

Touched up the Wine Tab

c80b205

* Added button for explorer with WineD3D * Changed SetWin7 to two buttons to set windows type quickly

Switch to using XIVLauncher.Common.Platform enum

c5331d0

Added some code in Distro for MacOS & FreeBSD.

f8a236b

rankynbass added 2 commits June 30, 2023 21:44

Minor cleanup of code and rewording some options.

2334b28

* Make Hud options a bit more clear * Add warning for MangoHud+ReShade+Dalamud

Update submodule

ccf30a7

rankynbass requested a review from marzent July 1, 2023 05:05

rankynbass added 5 commits June 30, 2023 22:51

Fixed flatpak path for mangohud

3b07474

Changed default wine to 7.10

92a7cff

Update WineVersion descriptions

7ab0524

Update submodule

22bfcbb

Update Distro.cs to correct namespace.

387d17d

rankynbass mentioned this pull request Jul 26, 2023

Compatibility rework 2 #64

Open

rankynbass closed this Jul 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compatibility rework #59

Compatibility rework #59

rankynbass commented Jun 24, 2023 •

edited

Loading

Blooym left a comment

Blooym left a comment

Blooym Jun 25, 2023

marzent commented Jun 25, 2023 •

edited

Loading

Blooym commented Jun 25, 2023

marzent commented Jun 25, 2023

Blooym commented Jun 25, 2023

marzent commented Jun 25, 2023

rankynbass commented Jun 25, 2023

marzent commented Jun 25, 2023

rankynbass commented Jun 25, 2023

marzent Jun 27, 2023

rankynbass Jun 29, 2023

marzent Jun 27, 2023

marzent Jun 27, 2023

marzent Jun 27, 2023

marzent Jun 27, 2023

rankynbass Jul 1, 2023

marzent Jun 27, 2023

marzent Jun 27, 2023

rankynbass Jul 1, 2023

marzent Jun 27, 2023

rankynbass Jun 29, 2023

marzent Jun 29, 2023

marzent Jun 27, 2023 •

edited

Loading

marzent Jun 27, 2023

rankynbass Jun 29, 2023

rankynbass commented Jun 29, 2023

rankynbass commented Jul 26, 2023


		public static bool IsFlatpak { get; private set; }

		public static void GetInfo()

Compatibility rework #59

Compatibility rework #59

Conversation

rankynbass commented Jun 24, 2023 • edited Loading

Blooym left a comment

Choose a reason for hiding this comment

Blooym left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marzent commented Jun 25, 2023 • edited Loading

Blooym commented Jun 25, 2023

marzent commented Jun 25, 2023

Blooym commented Jun 25, 2023

marzent commented Jun 25, 2023

rankynbass commented Jun 25, 2023

marzent commented Jun 25, 2023

rankynbass commented Jun 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marzent Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rankynbass commented Jun 29, 2023

rankynbass commented Jul 26, 2023

rankynbass commented Jun 24, 2023 •

edited

Loading

marzent commented Jun 25, 2023 •

edited

Loading

marzent Jun 27, 2023 •

edited

Loading