Broken map loading on Linux

[NY00123]

This is essentially the same problem as the following one for NBlood: https://github.com/nukeykt/NBlood/issues/694

As in NBlood's case, the switch -usecwd does not help with loading a user map.

I can load a SF2, albeit navigation in the menu starts from /, rather than ~/.config/notblood or the cwd. It'll also load if mus_sf2_bank is manually edited to have just the relative path within ~/.config/notblood.

As in NBlood's case, it's probably related to the value of pathsearchmode.

[tmyqlfpir]

I've attempted to remedy this issue in commit 866e2fb3b5f596069eda36453007c859557cdc5f, please confirm if this did do anything as I do not have a linux environment to confirm.

[NY00123]

Thanks for the attempt. It unfortunately didn't resolve the problem on its own, but see the rest of this message. There's actually not one but two problems in NotBlood (probably the same as in NBlood).

1. When I originally inspected it, the value of pathsearchmode was 1 on call to levelAddUserMap, so its value didn't change. As a reminder, it was indirectly set to 1 by GTK startup code. But, when configured to not show the startup window on launch, pathsearchmode is actually not set to 1 when I get to the menu. In such a case, I can properly select an SF2 from the menu, and it at least initially appears to be the case for maps as well; You don't start at /.

The difference from EDuke32 in terms of pathsearchmode is as follows. In EDuke32 and NotBlood altogether, CONFIG_ReadSetup sets pathsearchmode to 1 and then to 0. If called, it's then indirectly set to 1 (with no reset) via the GTK implementation of startwin_run.

However, EDuke32 also has a second call to CONFIG_ReadSetup that indirectly resets pathsearchmode to 0. Here's the commit with its addition: 9df3042826e0b1ff4c45bcc4ad56751a93151663

2. But, it wasn't just that. The map file was still not loaded. levelAddUserMap changed its file extension to ".DEF", and later simply removed the extension. This trimmed name was copied to pLevelInfo->Filename and then to gGameOptions.zLevelName. The last copy was repeated from levelSetupOptions later, after showing video clips. At the end, file extension ".MAP" was re-appended via dbReadMapCRC -> Resource::Lookup -> Resource::AddExternalResource.

This was essentially a problem of case sensitivity. If the actual file ends with lowercase .map, it can't be found using the uppercase extension .MAP while file names are treated as case-sensitive. Even when the command-line parameter -map is used, this is the case. This hasn't been a problem with Duke3D ports.

Hopefully, this gives more information. I tried asking about pathsearchmode in the past and didn't get a clear answer - it's possible that it should simply be 0 by default.

[tmyqlfpir]

This was not an issue of pathsearchmode then, as it's use is to set the working directory as the same directory as the executable. I did identify one possible issue of case sensitivity and have pushed a change to that code (83a5d7610cafaf0920629abc2f37be4b3c200348) - as far as I can see the internals of Blood and it's resource manager expect uppercase filenames (likely a DOS holdover). Please test the latest build when you can, thank you.

[NY00123]

Thanks for giving it another try. The map was still not loaded. The root cause of the problem seems to be that the relevant function (dbReadMapCRC) gets the relative path to the map with the file extension already trimmed, so it isn't known at this point.

While debugging, I saw that the exe tried opening bdbrosmaps/Bloodbath/bbarena.MAP and BDBROSMAPS/BLOODBATH/BBARENA.MAP altogether. The actual map name/path was bdbrosmaps/Bloodbath/bbarena.map. Note that I didn't use the most up-to-date version of the map pack, but this was otherwise the situation.

I'll see in EDuke32's own copy of startgtk.game.cpp if pathsearchmode should indeed be reset. If yes, the change can then be backported into NBlood and NotBlood (and NetDuke32).

[tmyqlfpir]

I've made another attempt, which is allowing case insensitive file loading (09c16095e87b745a5ec9f545d656b24a58d0105f). Please try this and see if it fixes the issue.

[NY00123]

Thanks again for further giving this a try (I used 837206ee37e98fc90fadad3eb3c921ea34c7f5b9). There's still no change, unfortunately.

I'm not sufficiently familiar with the internals of file system(s) management, especially in Blood's game code, but here's a good way to check if a fix of yours has a potential even while testing on Windows. Basically, you can check if any filename under a call to kopen4load ends with ".map" instead of ".MAP".

By the way, regarding pathsearchmode, I've opened an MR for the matching code in EDuke32: https://voidpoint.io/terminx/eduke32/-/merge_requests/252

[tmyqlfpir]

I've tested this on a virtual machine with lubuntu, and I have identified the issue in question. I'm somewhat baffled, as Linux custom map support has been broken for years now.

Internally, Blood's resource manager (assuming the reversed code is accurate) uses a crc32 hashing method with uppercase filenames. This is used for the RFF binary to load levels, and has been hackily extended to also work with external folders (something which may not have been part of the original DOS binary). The issue is, as soon as this is used on a case sensitive filesystem such as Linux, it'll fail to load external files that do not use uppercase characters. If you rename the map file to BDBROSMAPS/BLOODBATH/BBARENA.MAP it'll work fine, as that is what it will attempt to load.

The solution is to either run additional checks for the existing file, then use that case sensitive filepath before calling kopen4load, or to redo all the filepath code for Blood so it can handle case sensitivity. Both which are outside the scope of this fork. This is not even touching upon the lack of unicode character support which has been an issue plaguing build/EDuke32 from the start.

For now, rename custom maps to use uppercase filenames and wait for upstream to fix this.

[NY00123]

Thanks for having a further look in a VM.

I had no problem with loading maps ending with .MAP and .map using BuildGDX and Raze; Albeit I think that BuildGDX rewrote or replaced the file I/O implementation; Raze knowingly replaced it, reusing shared GZDoom code and merging Blood's 3 filesystems (directory, blood.rff and sounds.rff) into a single one at an early stage.

Regarding NBlood/NotBlood, it looks like the problem is just with the .MAP file extension. I've tested now and had no problem with opening bdbrosmaps/Bloodbath/Body2012.MAP from NotBlood, even with the various lowercase letters.

I didn't actually get to play Blood a lot, relatively speaking, so I wasn't doing that many tests with NBlood on Linux during its earlier days - clearly not with user-made contents. Not having many non-Windows users must explain the situation.

[tmyqlfpir]

All extensions are assumed to be uppercase, it does attempt to load the requested filepath, then on fail it'll reattempt in uppercase. The other ports did the right thing by using modern methods of file handling, leaving NBlood using authentic, original methods that made sense in a DOS environment from 1997.

The correct solution would be to properly parse the filepath and remove the harcoded extension checks used throughout Blood.

[tmyqlfpir]

I've pushed an update that will hopefully stomp this issue out once and for all. This also includes by default turning on usecwd by means of including a user_profiles_disabled dummy file as mentioned here. I don't see a reason to use a profiles directory by default, as it breaks modding support and loading map files from the menu.

Also added a new file opening method only used by Resource::AddExternalResource() which uses case insensitive filename access. This fixes loading maps from linux, as well as mods that use custom resources that were previously broken due to non-uppercase file extensions.

I've also merged your PR for EDuke32, which fixes the bug where every top level directory on linux is shown when attempting to load a user map from the menu.

Please test and confirm this works on your end, thank you.

[NY00123]

That's been a bunch of work - thanks for going through it again! To begin with, I originally considered checking if the game code itself can be modified to not omit the file extension and use it as-is.

I do confirm that map loading works in a case-sensitive manner, at least for the map file itself; The latter is actually closer to Raze's behaviors for the map file as passed from the command-line.

On the other hand, this also included changes to source/build/*/vfs.*, which reside in engine side. I asked EDuke32 devs for an opinion. Right now, due to Doom64hunter's WIP add-on manager for EDuke32 still being there in a separate dev branch, the preference is to not make filesystem changes.

Another concern brought up is that case-sensitivity may depend on the system locale. This also covers the function (B)tolower.

I feel like adding user_profiles_disabled as a default for non-Windows platforms is less desirable. With EDuke32 and NBlood, and also other ports and games, the default behaviors are to use a directory like ~/.config/notblood for the settings.

With user profiles enabled, I could load user maps from a subdir (bdbrosmaps/Bloodbath) when present under ~/.config/notblood, as I expected. Without a subdir, a map could also be loaded from where I started NotBlood. I otherwise couldn't load it, and this looked like the main difference from EDuke32, but using ~/.config/notblood was still more consistent with other ports and programs.

[tmyqlfpir]

That's been a bunch of work - thanks for going through it again! To begin with, I originally considered checking if the game code itself can be modified to not omit the file extension and use it as-is.

Without rewriting all ChangeExtension() and Lookup() access, this solution is the easier option and guarantees it'll detect matching filenames with different cases. Of course, if someone does do the heavy lifting upstream I'd be happy to revert this hacky solution - for now it works.

Another concern brought up is that case-sensitivity may depend on the system locale. This also covers the function (B)tolower.

I feel like adding user_profiles_disabled as a default for non-Windows platforms is less desirable. With EDuke32 and NBlood, and also other ports and games, the default behaviors are to use a directory like ~/.config/notblood for the settings.

As I do not main Linux (yet) I'll trust your recommendation and revert the changes.

If this is considered 'fixed' please close the ticket. Thanks again for the feedback and testing.

[dibollinger]

The usage of .config is a convention, and is useful when the binary is available through a package manager, or when multiple users are using the same tools. The binary is in that case usually placed in a section where the user has no write access, so stuff needs to be written to a user-specific directory.

Here's the definition of this convention

https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

But technically, eduke32 isn't really fully compliant either:

$XDG_DATA_HOME defines the base directory relative to which user-specific data files should be stored. If $XDG_DATA_HOME is either not set or empty, a default equal to $HOME/.local/share should be used.

$XDG_CONFIG_HOME defines the base directory relative to which user-specific configuration files should be stored. If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.

$XDG_STATE_HOME defines the base directory relative to which user-specific state files should be stored. If $XDG_STATE_HOME is either not set or empty, a default equal to $HOME/.local/state should be used.

$XDG_CACHE_HOME defines the base directory relative to which user-specific non-essential data files should be stored. If $XDG_CACHE_HOME is either not set or empty, a default equal to $HOME/.cache should be used.

Right now, everything ends up in the .config directory. Ideally, files like texturecache and texturecache.index should end up in the .cache directory, map files should go into .local/share/, etc.

But in short, yes, the .config directory in eduke32 should be preserved for Linux.

[tmyqlfpir]

EDuke32 isn't really fully compliant either

Which is why I was confused in the first place. Sometimes EDuke32 respects the config path, and other times it just outputs right into the current working directory.

[NY00123]

ok, I think that this issue can be closed for now. As in EDuke32's case, the code adding the kopen4load variant will probably not be added to the NBlood repository, either; The situation might change after the add-on manager for EDuke32 will be ready.

Thanks again for debugging!

[tmyqlfpir]

I've since reverted the case sensitive hacks made to vfs as NotBlood was starting to have different behavior for loading files from an absolute game directory path. Ideally this should be addressed upstream with a proper extension handling instead of the brute force method I attempted here. Sorry for the issue but my goal with NotBlood is to remain lockstep with any changes made upstream, ensuring both mod and command line argument compatibility - even if this means reverting hack fixes I've made downstream.