KSP-ModularManagement / KSPe

Extensions and utilities for Kerbal Space Program
http://ksp.lisias.net/add-ons/KSPAPIExtensions
Other
11 stars 5 forks source link

Dont get screwed by `System.IO.Directory.GetCurrentDirecrtory()` again. #37

Closed Lisias closed 1 year ago

Lisias commented 1 year ago

as the title says

Lisias commented 1 year ago

WiP

Sem título

Lisias commented 1 year ago

WiP

Sem título2

Lisias commented 1 year ago

No matter what I try to do, I can't reproduce a test bed where the System.IO.Directory.GetCurrentDirectory() misbehave, so I can't be sure about the code.

I need more information about the environment where this thing happens.

Lisias commented 1 year ago

Ah! Fellow Kerbonaut TurtleMountain diagnosed the trigger! It's CKAN's launcher:

Since on Windows, usually the File System is case insensitive, CKAN developers didn't normalised the pathname before using it. CMD, PowerShell and even CygWin, apparently, normalise the pathname before applying it into the pwd.

I can't say it's a CKAN's bug - the thing is working on Linux and MacOS (I think - it not, then it would be a bug), it works on Windows after all, and it's Windows's fault the huge mess Microsoft did by leaving to the Application the responsability of following standards. But since NTFS and ReFS supports case sensitive filenames (as an option), it would not be a bad idea on normalising them to match whatever is on the filesystem. It would make easier to keep cross-operating system compatibility.

In a way or another, the commit above really solved the problem - I just didn't have how to check the fix until this moment. Evidences below, using an interim release of KSPe and the current (buggy) release of KSP-Recall. Look for KSPe.IO.Directory.GetCurrentDirectory() , it is the evidence of the fix.

[LOG 22:21:49.692] [KSPe] System.IO.Directory.GetCurrentDirectory() : c:\program files (x86)\steam\SteamApps\common\Kerbal Space Program
[LOG 22:21:49.692] [KSPe] KSPe.IO.Directory.GetCurrentDirectory() :   C:\Program Files (x86)\Steam\steamapps\common\Kerbal Space Program\
[LOG 22:21:49.693] [KSPe] DETAIL: pwd:                        c:\program files (x86)\steam\SteamApps\common\Kerbal Space Program\
[LOG 22:21:49.693] [KSPe] DETAIL: origin:                     C:\Program Files (x86)\Steam\steamapps\common\Kerbal Space Program\
[LOG 22:21:49.695] [KSPe] DETAIL: KSPUtil.ApplicationRootPath C:/Program Files (x86)/Steam/steamapps/common/Kerbal Space Program/KSP_x64_Data/../
[LOG 22:21:49.695] [KSPe] DETAIL: KSPe.IO.Path.AppRoot        C:\Program Files (x86)\Steam\steamapps\common\Kerbal Space Program\
[LOG 22:21:49.695] [KSPe] DETAIL: origin:                     C:\Program Files (x86)\Steam\steamapps\common\Kerbal Space Program\
Lisias commented 1 year ago

Closing it, as it's working as expected.

Lisias commented 1 year ago

DAMN! I just realised that I didn't used RealPath on most of the new KSPe.IO.Directory.* calls. This will allow KSPe clients to jailbreak the KSP's filesystem hierarchy. I need to rework this.

Additionally… The SetCurrentDirectory should probably be shortcircuited to an exception - no one should be allowed to change the pwd on KSP, as this will break things for sure.

I'm also created a problem, as GetCurrentDirectory needs to return a full pathname, breaking the standard of returning always partial pathnames rooted on Origin() AppRoot(). This is only happening because I need the pwd check on KSP-Recall — at least, until KSPe itself is not widely available to the public, and so KSPe will be in charge os such check. I need to find a solution for this dilema ASAP, as it's not wise to break paradigms on public interfaces!

Lisias commented 1 year ago

AND… another missed use case was found.

[KSP-Recall] Version 0.3.0.7 /L running on KSP 1.12.3 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 35)

[KSP-Recall] ERROR: pwd != Application Root! -- pwd=E:\Spiele\Kerbal Space Program\ ; AppRot=e:\Spiele\Kerbal Space Program\ at error:0 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 35)

[KSP-Recall] "Houston, we have a problem!" about "Your 'pwd' doesn't match KSP's 'Application Root'!" was displayed 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 35)

How in hell the KSPUtil.ApplicationUtil is returning a lower case drive letter?

Lisias commented 1 year ago

Fellow Kerbonaut Gordon Dry nailed it.

Launching things from CMD appears to normalise the pwd, but not the executable pathname!!! ##facePalm

Oh, well. O Mundo gira e a Lusitânia roda, indeed. E vamo que vamo...

Lisias commented 1 year ago

Oww, Krappp!!!

Thats the history: if you launch KSP from a batch file on Windows using only the executable name, KSP will set the 'AppRoot' using the "correct" casing on the current directory (or perhaps it uses whatever was set before, and the cd command does the trick itself).

if you launch KSP using a full pathname with the wrong case, then the 'AppRoot' is set using whatever you had typed!

For example, the following batch file works fine on rig:

@echo off
cls
KSP_x64.exe -force-d3d11 -single-instance

But the following borks on the current KSPe release:

@echo off
cls
c:\users\lisias\desktop\gordondry\KSP_x64.exe -force-d3d11 -single-instance

Where the file is located on a directory called C:\Users\Lisias\Desktop\GordonDry.

KSP is borking because it is trusting the Operating System.

Well, one more lesson learnt - and now we know why is so hard to have games running on non Windows machines. Windows is a utter and terrible pile of shit!

Lisias commented 1 year ago

Implemented on commits:

Closing it. Hopefully for the last time! :)