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

Modular installer part1 #20440

Merged
merged 3 commits into from
Nov 16, 2022
Merged

Conversation

IceReaper
Copy link
Contributor

@IceReaper IceReaper commented Nov 7, 2022

Part 1 #20440
Part 2 #20445
Part 3 #20439

What we have here:

  • InstallShield CAB format now also supports version 5. I took the logic from UnShield. Tested to work on KKnD Xtreme demo data1.cab
  • ZipFiles are now determined by their zip header, instead of their filename. Otherwise downloaded temp files for example wont be extractable, as they have no file extension.
  • And the "biggest" thing here: Installer file sources are now case insensitive. Any game which is released on Windows or MacOS will be case insensitive anyway. This would only lock out pure linux games, which have two identical filenames which only differ in their capitalization. So while theoretically possible, this most likely never occurs as no game dev goes linux only AND uses a filename with different capitalization twice.

OpenRA.Game/FileSystem/FileSystem.cs Outdated Show resolved Hide resolved
OpenRA.Game/FileSystem/FileSystem.cs Outdated Show resolved Hide resolved
OpenRA.Game/FileSystem/FileSystem.cs Outdated Show resolved Hide resolved
@IceReaper IceReaper force-pushed the modular-installer-part1 branch 2 times, most recently from d769e7e to 0f35afc Compare November 7, 2022 19:26
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

A couple of small issues with the latest amendment. I haven't yet had the opportunity to test the installer ingame.

OpenRA.Game/FileSystem/ZipFile.cs Outdated Show resolved Hide resolved
@pchote
Copy link
Member

pchote commented Nov 8, 2022

The /./ issue you mentioned in #20439 is a problem here:

On bleed:

Using installer `cnc95: C&C Gold (GDI or Nod Disc, English)` of type `Disc`:
Copying /Volumes/GDI95/./DESERT.MIX -> /Users/paul/Library/Application Support/OpenRA/Content/ra/v2/cnc/desert.mix

On this PR:

Using installer `cnc95: C&C Gold (GDI or Nod Disc, English)` of type `Disc`:
Copying  -> /Users/paul/Library/Application Support/OpenRA/Content/ra/v2/cnc/desert.mix
System.ArgumentNullException: Path cannot be null. (Parameter 'path')
   at System.IO.Strategies.FileStreamHelpers.ValidateArguments(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
   at OpenRA.Mods.Common.Widgets.Logic.InstallFromDiscLogic.<>c__DisplayClass42_0.<InstallFromDisc>b__1() in /Users/paul/src/OpenRA/OpenRA.Mods.Common/Widgets/Logic/Installation/InstallFromDiscLogic.cs:line 286
Deleting /Users/paul/Library/Application Support/OpenRA/Content/ra/v2/cnc/desert.mix

@pchote
Copy link
Member

pchote commented Nov 8, 2022

Tested and otherwise works as expected on macOS and Linux, so 👍 once the fix above is applied.

Copy link
Contributor

@AspectInteractive2 AspectInteractive2 left a comment

Choose a reason for hiding this comment

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

The design looks fine. I can't be sure of the installation specifics as this isn't my area of expertise, but as this has been tested successfully and reviewed by pchote I also approve.

@PunkPun
Copy link
Member

PunkPun commented Nov 16, 2022

Merging as approved by both @pchote and @AspectInteractive2

@PunkPun PunkPun merged commit 8ae5383 into OpenRA:bleed Nov 16, 2022
@PunkPun
Copy link
Member

PunkPun commented Nov 16, 2022

Changelogs

@IceReaper IceReaper deleted the modular-installer-part1 branch November 17, 2022 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants