Installer should not hardcode the installdir path

[whindsaks]

This changes $INSTDIR to respect InstallDirRegKey. If not present, the default is the drive of %ProgramFiles% instead of forcing c:.

[dreimer1986]

This is done on purpose as Program Files does not provide the wanted access rights and another not needed space in path.

[whindsaks]

This is done on purpose as Program Files does not provide the wanted access rights and another not needed space in path.

And I did not change that, only the drive letter is taken from ProgramFiles instead of forcing C:\.

[JoachimHenze]

Personally I do think the current code is more readable. And the installer allows you to change it to something else still. So calling that "hardcoded" is a bit stretched imho. I do vote for not merging this PR.

[whindsaks]

So calling that "hardcoded" is a bit stretched imho

It is. The .nsi uses InstallDirRegKey but that can never actually work as intended because it will always be overridden to C:\RosBE, this is clearly a bug and comparing $InstDir to an empty string is the correct way to handle that.

If you think I'm incorrect then surely you must be advocating for the InstallDirRegKey line to be removed?

[dreimer1986]

I did not say it's invalid completely, but where does InstallDirRegKey lead to? The former installation path, right? So this is fine and your fix, too. But the changing of the default path is not. We had quite some reasons to move it to C:\ root.
So keeping line 58 on "C:\RosBE" is fine for me. And if you dislike that you need to work with "ReadEnvStr $R0 SYSTEMDRIVE" in .onInit then I guess to have at least a valid system drive root folder to use.

[whindsaks]

But the changing of the default path is not.

It does no such thing for any user where ProgramFiles is on drive C:\ (99% percent of users?).

And if you dislike that you need to work with "ReadEnvStr $R0 SYSTEMDRIVE" in .onInit then I guess to have at least a valid system drive root folder to use.

This PR does essentially that except it grabs the drive letter from ProgramFiles, it does not change "C:\RosBE" as the default for anyone that are installing ROS to C:\. I can change it to grab the drive letter from the result of $SysDir (GetSystemDirectory) if that makes you feel better. Getting the result from a Windows function is better than from an environment string that might not exist.

[dreimer1986]

My NSIS is rusty as you see. If that's the case, I am fine with the pull.