search

X2CommunityCore / xcom2-launcher

The Alternative Mod Launcher (AML) is a replacement for the default game launchers from XCOM 2 and XCOM Chimera Squad.
GNU General Public License v3.0
354 stars 52 forks source link

Should inform user that AML needs XComEngine.ini to be writable #201

Closed pepoluan closed 4 years ago

pepoluan commented 4 years ago

Technical Data

AML version: AML 1.4.0-beta d8206f1fc3e0afaf3d37ce5a6b1ebaf88b41ddcc

Game: XCOM: Chimera Squad

Issue Description

Upon starting AML for the first time, it complained that it cannot find the location of mods directory for the game from the XComEngine.ini file.

Checking the log, I see System.UnauthorizedAccessException and I immediately know that the problem was likely AML opening XComEngine.ini for read/write access while I have set the file to be read-only.

I closed AML, removed the Read Only attribute from XComEngine.ini, and reopened AML, and now it works.

I suggest trapping that Exception and give a more informative error message to the user, e.g. "Please do not set the XComEngine.ini file Read-Only" or something to that effect.

RevZero commented 4 years ago

Thanks for reporting, but please consider attaching AML.log when posting bug reports.

Out of interest, why did you write protect the file in the first place?

Anyway, as you pointed out the Exception gets handled and displays:

2020-05-14 21_28_48-Error

Which is exactly what you have done and found details about what went wrong. I don't really see that it is necessary to change this as this should be a rare issue.

pepoluan commented 4 years ago

Thanks for reporting, but please consider attaching AML.log when posting bug reports.

Will do next time, sorry about that.

Out of interest, why did you write protect the file in the first place?

Was trying to debug Chimera Squad crashes by selectively enabling the logging and I don't want the settings to change while I was doing so.

Which is exactly what you have done and found details about what went wrong. I don't really see that it is necessary to change this as this should be a rare issue.

True, it's not really necessary. But perhaps a quick check on read-only status and aborting? Because after that, AML continued with the launch, though showing no mods. I'm a bit concerned that someone less technically inclined will just barge on and cause problems as .ini files get out of sync (if that is possible).

But that's just a small concern from me. I'd understand if you think the effort is not worth it :-)

RevZero commented 4 years ago

Yeah its just that DetectModDirs() currently just returns null if something went wrong, which is communicated by the dialog. The calling method/class doesn't "know" what exactly went wrong and I prefer to keep the logic separated at this point.