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

NSIS: Update installer scripts, bug fixes, added some new goodies #1699

Merged
merged 8 commits into from
May 16, 2017

Conversation

MrCK1
Copy link
Member

@MrCK1 MrCK1 commented Dec 15, 2016

Summary of changes:

Main Installer Changes:

  • Conducted compatibility testing for installer and build scripts in NSIS 3.01

  • Implemented uninstall script for future version detection. If an older or equivalent PCSX2 version is detected, the user is prompted to silently uninstall/overwrite or abort the installer. (The function is not compatible with any current stable releases because they lack the new registry string required for version detection.)

  • We can launch PCSX2 directly from the installer now 👍

You can try the new installer(s) with version detection here

MakeNSIS:

  • Suppressed errors related to unused plugins
  • Replaced some functions
  • Removed an leftover parameter.

Uninstaller:

  • Added section that allows users to keep just memory cards/savestates.
  • Fixed an issue with a registry key left behind by the uninstaller.
  • Fixed an issue with .ini files being deleted unintentionally
  • Removed obsolete function

Misc:

  • Updated copyright, added icon to installer
  • Reformatted installer instructions and added additional sections, fixed broken VC++ URL.

Copy link
Member

@turtleli turtleli left a comment

Choose a reason for hiding this comment

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

Indentation seems a bit weird. And I see plenty of trailing whitespace.

!insertmacro UNINSTALL.LOG_OPEN_INSTALL
File ..\bin\Cheats\*
!insertmacro UNINSTALL.LOG_CLOSE_INSTALL
CreateDirectory "$INSTDIR\Cheats"

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Quit
${Else}
DetailPrint "Not the same version"
${EndIf}

This comment was marked as spam.

!endif

SectionEnd

This comment was marked as spam.

@ramapcsx2
Copy link
Member

Thanks a lot for these improvements!

@@ -53,20 +53,13 @@
; in order to run (including CDVD!) -- and really there should be more but we don't have working
; SPU2 null plugins right now.

File ..\bin\Plugins\GSnull.dll
;File ..\bin\Plugins\GSnull.dll

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

;File ..\bin\Plugins\SPU2null.dll
File ..\bin\Plugins\USBnull.dll
File ..\bin\Plugins\DEV9null.dll
File ..\bin\Plugins\FWnull.dll
File ..\bin\Plugins\CDVDnull.dll
;File ..\bin\Plugins\CDVDnull.dll

This comment was marked as spam.

This comment was marked as spam.

@@ -18,8 +18,8 @@

SetOutPath "$INSTDIR"
!insertmacro UNINSTALL.LOG_OPEN_INSTALL
File /oname=${APP_EXE} ..\bin\pcsx2.exe
;File /nonfatal /oname=pcsx2-dev.exe ..\bin\pcsx2-dev.exe
File ..\bin\pcsx2.exe

This comment was marked as spam.

@@ -30,24 +30,28 @@ Section "Un.Program and Plugins ${APP_NAME}"
SectionEnd

; /o for optional and unticked by default
Section /o "Un.Configuration files (Program and Plugins)"
Section /o "Un.Configuration files (PCSX2, GSdx, SPU2-X, ect.)"

This comment was marked as spam.

SectionEnd

; /o for optional and unticked by default
Section /o "Un.User files (Memory Cards, Savestates, etc)"
Section /o "Un.User files (Cheats, logs, snaps, etc)"

This comment was marked as spam.


;This check for older versions (pre 1.6.0) without InstalledVersion string will bypass this section
${If} $R1 == ""
DetailPrint "InstalledVersion string not found, skipping version check"

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@MrCK1
Copy link
Member Author

MrCK1 commented Dec 20, 2016

I went ahead and updated the Uninstaller labels with FlatOut's suggestions. I also merged the remaining code from SharedRedtape into SectionUninstaller; and the deleted the unused SVN templates

@FlatOutPS2 @ramapcsx2
Do we have any thoughts on what we want to do with the null plugins? If we include them in the installer, it would look something like this:

installlersample

@ramapcsx2
Copy link
Member

"Optional Plugins" looks like a good solution to me. As well as having them disabled by default.

@FlatOutPS2
Copy link
Contributor

I'm in favour of not including 'em at all. There are always users who click everything and then complain that they have no graphics, no sound or whatever issues Jackun's plugin can cause. The few people who really want to use these plugins can easily find them on the website.

@refractionpcsx2
Copy link
Member

I'm inclined to agree with @FlatOutPS2 at least as far as the null plugins go. The useful USB plugins should be included along with any additional pad plugins like pokopom IMO.

@FlatOutPS2
Copy link
Contributor

I'm inclined to agree with @FlatOutPS2 at least as far as the null plugins go. The useful USB plugins should be included along with any additional pad plugins like pokopom IMO.

If you add 'em, you need to support 'em too. And the USB plugins are functional at best. They're not very user friendly and still suffer many issues. And we already have three PAD plugins, no sense in adding anymore.

@refractionpcsx2
Copy link
Member

If you add 'em, you need to support 'em too. And the USB plugins are functional at best. They're not very user friendly and still suffer many issues.

True but they do add a lot of functionality people like.

And we already have three PAD plugins, no sense in adding anymore.

Do we? In my git repo I have xpad and LilyPad

@FlatOutPS2
Copy link
Contributor

True but they do add a lot of functionality people like.

And they can download those plugins if they wish. It's not like those plugins see a lot of maintenance. I posted an issue including a possible solution for the USB mic plugin, but the dev never even responded to any of the posts in the thread @bositman made.

Do we? In my git repo I have xpad and LilyPad

And OnePad for Linux. I don't see the use of more PAD plugins. Just gives more room for new issues. We could possibly replace xpad with Pokopom. I'm not sure anyone actually uses xpad regularly. :p

@refractionpcsx2
Copy link
Member

It depends which of the two is going to be more useful :P

@MrCK1
Copy link
Member Author

MrCK1 commented Dec 22, 2016

I guess you guys can figure out which PAD plugins we want to include and whether or not to include Jackun's USB plugin.

For the future, I was also thinking about replacing the FAQ .pdf with a link to a configuration on the wiki. The current Windows guide was written for 1.2.1 and contains obsolete information. The readme could possibly be simplified to a .txt file containing the patch notes.

@turtleli
Copy link
Member

I don't think xpad should be included - it's not as compatible as LilyPad. If xpad is used and you try to play Xenosaga 2, you get the following message:

The DUALSHOCK®2 Analog controller has been removed.

Please insert a DUALSHOCK®2 analog controller into controller port 1 to continue.

@lightningterror
Copy link
Contributor

lightningterror commented Dec 23, 2016

Hmm is it a good idea to include generic usb gamepad drivers ?

In "Other" category.(not installed by default) The latest version is 3.70a if I'm not mistaken.

What do you guys think about it ?

If not then a great alternative would be adding links on the website. Of some optional but required installation packages like DirectX , visual 2015 , different types of gamepad drivers ..etc that are required for pcsx2 to run.

@MrCK1
Copy link
Member Author

MrCK1 commented Dec 24, 2016

Generic USB drivers are probably best left to be handled by the user's operating system. It's also much easier to bundle DirectX packages with the installer than to host them separately and introduce additional user error to the equation ;)

@toehead2001
Copy link
Contributor

It would be a good idea to make it scale properly on HiDPI.

Add this to the pcsx2_full_install.nsi file:
ManifestDPIAware true

@MrCK1
Copy link
Member Author

MrCK1 commented Dec 28, 2016

I'll add that in when I get a chance ;)

@MrCK1
Copy link
Member Author

MrCK1 commented Dec 28, 2016

@turtleli Looking at your previous comment, maybe you should also remove xpad from the VS solution?

@turtleli
Copy link
Member

Looking at your previous comment, maybe you should also remove xpad from the VS solution?

I suppose it could be removed from the solution. Or moved to legacy. Dunno.

@@ -38,7 +38,8 @@ SetCompressorDictSize 24
; The name of the installer
Name "${APP_NAME}"

OutFile "output\pcsx2-${APP_VERSION}-${OUTFILE_POSTFIX}.exe"
; Output the installer to the nsis folder (in git repo)
OutFile "pcsx2-${APP_VERSION}-${OUTFILE_POSTFIX}.exe"

This comment was marked as spam.

This comment was marked as spam.

@MrCK1
Copy link
Member Author

MrCK1 commented Dec 30, 2016

I just checked the current gitignore - I see what I forgot to do when I removed the svn templates earlier. I'll remove the directories tomorrow

@MrCK1
Copy link
Member Author

MrCK1 commented Jan 18, 2017

Here's a summary of the additional changes in the past few weeks:

  • Fixed a regression I introduced in the registry that caused random FTW configuration issues to occur after a reinstall/upgrade.
  • Reduced version detection into a single function and added a message reminding users to back up files before a silent uninstall.
  • Removed unnecessary description text and parameters.
  • Installation is now blocked on XP and support for high DPI screens has been added.

@MrCK1
Copy link
Member Author

MrCK1 commented Apr 10, 2017

@mirh I needed to push some changes to work on my other PR; but are there any more files besides the two you mentioned (XAudio2_7.dll and D3DCompiler_43.dll) that we need to look for on Win 8.0 and older? I'm asking because (if this is the case) I can skip the DXWebSetup if both are present on the user's system. 😃

@mirh
Copy link

mirh commented Apr 10, 2017

Well, that would be the point.
Anyway, those were just two examples. I could have said XInput1_3 too. One alone is good as the others.

@gregory38
Copy link
Contributor

Could we stop adding more goodies to this PR ? It would be nice to validate the already huge work, merge it. And then do another PR with more goodies.

Note that I'm also fine if people prefer to do it in one shot.

@MrCK1
Copy link
Member Author

MrCK1 commented Apr 10, 2017

OK, there's really nothing else to be added besides the new code for skipping DxWebSetup, but I'll leave this as-is for now. The changes in the last commit should address most of the remaining concerns.

@danilaml
Copy link

@MrCK1 you don't really need to skip DxWebSetup manually since it will do all the necessary detection by itself (try running it yourself after you already installed dx9).

@MrCK1
Copy link
Member Author

MrCK1 commented Apr 10, 2017

Well that's good then, not sure how I got so worried about skipping the package in the first place. 😛

@mirh
Copy link

mirh commented Apr 10, 2017

you don't really need to skip DxWebSetup manually since it will do all the necessary detection by itself

Doesn't it still need internet?

@danilaml
Copy link

@mirh to install dx, yes. It's Web Installer after all. But you'll need internet either way if you don't want to bundle dx9 with pcsx2 installer.

@mirh
Copy link

mirh commented Apr 10, 2017

But you'll need internet either way if you don't want to bundle dx9 with pcsx2 installer.

You also don't need internet if we detect in the first place the thing is not needed.

@danilaml
Copy link

@mirh it want burn your computer and kill your dog if there is no internet. It just won't install anything. So if you have no internet connection the result will be the same, whether you have dx9 installed or not.

@mirh
Copy link

mirh commented Apr 10, 2017

Assuming we can detect return codes, having no internet from dx setup should abort pcsx2 installation.

@danilaml
Copy link

@mirh should it? Can't pcsx2 work without dx9 redist?

@MrCK1
Copy link
Member Author

MrCK1 commented Apr 10, 2017

Don't forget this is only here for legacy systems.

@mirh I see little point in adding a code for detection. I don't think that a noticable speed improvement will be seen by searching the registry for .Dlls versus just letting the package abort itself.

@mirh
Copy link

mirh commented Apr 11, 2017

should it? Can't pcsx2 work without dx9 redist?

I'm not sure if we crash or just fallback to whatever, but at least no XAudio and no XInput is a pretty big deal imo.
Don't know about gsdx requirements then.

I don't think that a noticable speed improvement will be seen by searching the registry for .Dlls versus just letting the package abort itself.

It wasn't about speed. It was more about: "no ignorant left behind".
If this details bothers you, we can even discuss it separately sometime else.

@MrCK1
Copy link
Member Author

MrCK1 commented Apr 19, 2017

@gregory38 does this PR still need a review/anything before we merge it?

I'll resolve/add the registry detection to skip dxwebsetup in another PR later. Other than that, the scripts are fully functional as-is.

@gregory38
Copy link
Contributor

@MrCK1 lol don't ask me.

@turtleli @refractionpcsx2 @ramapcsx2
Can we merge this branch ?

ExecWait '"$TEMP\dxwebsetup.exe" /Q' $DirectXSetupError

${NSD_CreateLabel} 0 45 100% 20u "Finished installing Visual C++ Redistributables. Click Next to continue."
${NSD_CreateLabel} 0 45 100% 20u "Finished installing Visual C++ Redistributables. Click Next to continue."

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

${StrContains} "$4" "1.0.0" "$R7"
StrCmp $4 "" +1
${If} $R7 == ""
${EndIf}

This comment was marked as spam.

This comment was marked as spam.

; This will become the primary version check once more stable builds
ReadRegStr $R1 HKLM "${INSTDIR_REG_KEY}" "DisplayVersion"
${If} $R1 == ""
${EndIf}

This comment was marked as spam.

${StrContains} "$2" "1.4.0" "$R2"
StrCmp $2 "" +1
${If} $R2 == ""
${EndIf}

This comment was marked as spam.

${StrContains} "$3" "1.2.1" "$R3"
StrCmp $3 "" +1
${If} $R3 == ""
${EndIf}

This comment was marked as spam.

CopyFiles /SILENT /FILESONLY "$R3\Uninst-pcsx2-r5875.exe" "$TEMP"
ExecWait '"$TEMP\Uninst-pcsx2-r5875.exe" /S _?=$R3'
Delete "$TEMP\Uninst-pcsx2-r5875.exe"
Goto Done

This comment was marked as spam.

CopyFiles /SILENT /FILESONLY "$R5\Uninst-pcsx2-r4600.exe" "$TEMP"
ExecWait '"$TEMP\Uninst-pcsx2-r4600.exe" /S _?=$R5'
Delete "$TEMP\Uninst-pcsx2-r4600.exe"
Goto Done

This comment was marked as spam.

This comment was marked as spam.

ExecShell open "$TEMP\pcsx2-${APP_VERSION}-include_standard.exe"
Quit
${Else}
Goto +2

This comment was marked as spam.

true:
Goto SetUninstPath
false:
Quit

This comment was marked as spam.

SetOutPath "$DOCUMENTS"
CopyFiles /SILENT "$DOCUMENTS\PCSX2" "$DOCUMENTS\PCSX2_backup"
RMDir /r "$DOCUMENTS\PCSX2"
SetOutPath "$TEMP"

This comment was marked as spam.

This comment was marked as spam.


; Installing another version
MessageBox MB_ICONEXCLAMATION|MB_OKCANCEL "Another version of PCSX2 is already installed. The current configuration folder in Documents will be duplicated and renamed as PCSX2_backup. Click OK to uninstall PCSX2 or Cancel to abort the setup." IDOK SetUninstPath

This comment was marked as spam.

…ninstaller, automatically backup old configuration folder, misc. changes.

Automatically backup old configuration folder to avoid conflicts, misc. changes.

Added reviewed changes
@MrCK1
Copy link
Member Author

MrCK1 commented May 2, 2017

@turtleli I'm ready for another review :)

; Installing another version
MessageBox MB_ICONEXCLAMATION|MB_OKCANCEL "Another version of PCSX2 is already installed. The current configuration folder in Documents will be duplicated and renamed as PCSX2_backup. Click OK to uninstall PCSX2 or Cancel to abort the setup." IDOK SetUninstPath IDCANCEL false

false:

This comment was marked as spam.

@turtleli
Copy link
Member

turtleli commented May 6, 2017

I've reviewed what I can (well, I'm completely ignoring whitespace issues, otherwise it may never end). I doubt I'll be creating the final installer package when it's time to do so, so I'm not the right person to ask to merge this (I also didn't test this at all).

@gregory38
Copy link
Contributor

Let's merge the code. It won't create regression anyway and PR is already huge

@ramapcsx2
Copy link
Member

ramapcsx2 commented May 15, 2017

@turtleli @refractionpcsx2 @ramapcsx2
Can we merge this branch ?

This PR is so huge, it can't be managed anymore anyway.
Merge it and we'll go from there, making references back. Only if absolutely necessary.

Edit: Some more justification, I guess.
This improves about 10 things, leaves 2 things in limbo and maybe makes 1 thing worse. So yea :p

@gregory38 gregory38 merged commit e043822 into PCSX2:master May 16, 2017
@MrCK1 MrCK1 deleted the NSIS3.01_upgrade_CK1 branch May 17, 2017 02:07
@FlatOutPS2 FlatOutPS2 mentioned this pull request Feb 4, 2018
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.