Fix for mods not being properly reloaded on Windows from saved game

[dvantwisk]

Steam mods with lua scripts in the "scripts_modactive" directory of a mod were not being properly reloaded after the initial load. That is, the script-manager purges these scripts from dfhack.internal.scripts before reloading them (more specifically to register their onStateChange). This purge wasn't being done since the file path wasn't being recognized on windows, thus dfhack mods weren't being loaded in games from resumed saves (if DF wasn't shut down before then). This change simply adds the correct windows file path as a key to access the script. It may be better to standardize all OS's paths used as keys in dfhack.internal.scripts but it isn't clear to me the locations where all these files are being added, so this is the simple solution.

[myk002]

I moved the fix into the core function that is returning the incorrect string. Thank you for helping us track this down!

[dvantwisk]

@myk002
I'm trying to run a script on 51.11-r1.2 and it doesn't seem to conserve its state on Windows. This version should have the change you added. It does work if I use my fix. Can you double check to see if it works? It may be easier to just use the simple solution I originally had.

[myk002]

That commit was merged after 51.11-r1.2 was tagged. Try the testing branch to see if the issue persists there

[dvantwisk]

@myk002
Okay, just looked at it. The problem is that the keys are now all forwardslashed, but the script_paths are still backslashed. It can be fixed with this at line 55 after full path is initially declared:
full_path = string.gsub(full_path, "\\", "/")
This is a windows issue, of course.

[myk002]

Ah, are you saying that the paths returned from dfhack.filesystem.listdir_recursive have backslashes in them? The fix should be in that function, then. The intent of @ab9rf 's change is that backslashes should not make their way back to Lua.

[dvantwisk]

That seems to be what is happening. I guess I don't know exactly how you guys want to plug up all the window's backslash holes.

[ab9rf]

Ah, are you saying that the paths returned from dfhack.filesystem.listdir_recursive have backslashes in them? The fix should be in that function, then. The intent of @ab9rf 's change is that backslashes should not make their way back to Lua.

No, listdir_recursive generates a list of pathname objects, not a list of strings. The fix implemented in this PR is the correct fix: the transition should occur, using as_string, when a pathname is to be interpreted as a string, and not before.

Pathnames as pathnames should be represented in Lua as userdata objects wrapping a std::filesystem::path object, not as Lua strings, and should not be manipulatable as strings in Lua. If/when they need to be interpreted as strings, we can provide a Lua method for that (using the as_string function in Filesystem).

The issue goes beyond backslashes; Windows pathnames are encoded in UCS-2, not UTF-8. There are UCS-2 codepoints that have 0x2f (/) and 0x5c (\) as their second octet that are not slashes (e.g. 0x012f į, 0x015c Ŝ) , and a whole bunch that have 0x2c as their first octet (basically the entire Glagolithic and Coptic planes, which admittedly are unlikely to be used in a filename) and so using Lua's gsub function (which isn't even UTF-8 aware) as proposed will damage any pathname that contains one of those characters. I think our current code attempts to transcode Windows pathnames from UCS-2 to UTF-8 on the transition to Lua, which should avoid this issue, but I'm not sure we have all the ducks covered. It's far better to treat pathnames as opaque blobs and explicitly convert them to strings, using platform-aware tools, than to just assume that they're blithely interconvertible to strings (because they're not).

Perhaps we should just add the as_string call to df::path_identity::lua_read (instead of using u8string as we do now). The better solution would be to store the pathnames in the table as pathnames, not as strings, but there may be lifetime issues with this (someone has to own the filesystem::path objects as at present Lua cannot). This is something I'm working on making the Lua environment support. It's actually quite straightforward, just a matter of adding a __gc key to the metatable for the object that wraps a deleter for the C++ type, which will cause Lua to effectively own the object and be responsible for its deletion; the hard part is figuring out how to add this to the object metatable, which I haven't quite figured out because the code to create metatables for our Lua userdata objects is quite mazelike and basically undocumented.

I just checked the code for filesystem_listdir_recursive and it calls Filesystem::as_string so when this is called from Lua the Lua environment should get strings with forward-slashed names (since we currently don't support Lua owning a std::filesystem::path object opaquely). However, filesystem_listdir does not have the as_string conversion.

[myk002]

filesystem_listdir does not have the as_string conversion

bollocks, let me fix that then.

I'll get that change pushed to the testing branch.