FWGS / xash3d-fwgs

Xash3D FWGS engine
1.59k stars 242 forks source link

Relative path handling in liblist.gam is insufficient and differs from vanilla #1909

Open jarcen opened 19 hours ago

jarcen commented 19 hours ago

There is a better-than-typical metamod setup that involves putting it into game's root directory, like that:

D:\Games\Half-Life\metamod\metamod.dll
D:\Games\Half-Life\valve\liblist.gam

And into liblist.gam goes relative path to it like that: gamedll "..\metamod\metamod.dll"

It's better because there's no need to copy it into every mod directory. Easier to update and maintain.

Works in vanilla, but in Xash3D:

Error: LibraryLoadSymbols: couldn't load ../metamod/metamod.dll
Error: can't initialize ../metamod/metamod.dll: Failed to load library ../metamod/metamod.dll

Changing backslashes does nothing. I did some experiments and concluded that relative path handling is lackluster.

gamedll "dlls\metamod.dll" - Works. gamedll ".\dlls\metamod.dll" - Curiously, works. gamedll ".\.\dlls\metamod.dll" - When this doesn't.

These three are equivalent and valid paths in original engine. They all should work along with parent dir ..\ prefix.

a1batross commented 14 hours ago

Hm. We specifically remove upper directory access here, as none of user-facing DLLs are allowed to reference absolute paths or upper directories. While this works in GoldSrc, Xash in general quite strict about accessing absolute or upper paths through VFS as this might lead to undesired results.

If you want to reference in DLL from another game directory, basedir or falldir keys in gameinfo.txt could be used instead (Xash historically doesn't work with liblist.gam directly, it converts it to its own format in gameinfo.txt, see documentation).

Not only that, Metamod-R for example specifically looks for its own path when loading configuration files. I don't know about original Metamod, because it doesn't seem to be really useful.

jarcen commented 13 hours ago

I suspected possibility of a deliberate limitation, but why? Security? DLLs can execute arbitrary code anyways.

I solved it by setting fallback_dir to shared dir. Although, handling of gameinfo.txt is not convenient. Any edit to liblist.gam overwrites it and even Read Only file attribute doesn't preserve it. (Actually no, Read Only in Notepad++ doesn't set the attribute, Xash3D crashes instead if actual file attribute is set).

Why I bother with old liblist.gam, you might ask? Because not everything goes smoothly with Xash3D and I have to use both engines. That includes checking for bugs and comparing behavior when reporting stuff to be sure it's not my mistake.

Alright, my current problem is kinda solved. If you believe nothing needs to be improved about handling of these files, then you can close the issue. (For example, there's minor jank with version "4.1.2 Beta" becoming version "41.2" in gameinfo.txt, I'm not sure if I should make new issues for these minor things).

a1batross commented 12 hours ago

In case of DLLs, I believe it's less of a security issue but rather some assumptions on how GoldSrc loads libraries in original Xash.

In our case, it might cause a problem when the game info wants ../foo/bar.dll but game directory already has bar.dll in foo subdirectory. As we allow putting mod data in game directory custom subdirectory, it might instead get resolved relative to that subdirectory, which is that undesired effect I was talking about.

An alternative to this might be excluding DLLs from mod subdirectories like this, but I'm not sure if somebody might already rely on this feature. I'm using that myself to not clutter game directory with modded data, but never put DLLs there.

As about liblist.gam and gameinfo.txt, some time ago I planned disabling auto-generating gameinfo.txt and instead using liblist.gam directly, but again, not sure if somebody relies on gameinfo.txt being generated from liblist.gam. However, I've seen people mistakingly shipping outdated and broken liblist.gam in their mods and because users archiver software didn't preserve timestamps, the engine would regenerate gameinfo.txt and everything blows up.

jarcen commented 12 hours ago

gameinfo.txt looks redundant to me. liblist.gam seems to be allowing any key-value pair and original engine simply ignores anything unknown. It would be really great to disable auto-generation of gameinfo and instead use fusion of the two dictionaries: first we read key-values from liblist then read gameinfo if it's present. Any conflicting key-value pair from new dictionary overrides the one read from old dictionary. (Of course, mapping of keys like edicts->max_edicts would need to be done internally to fuse dictionaries correctly.)

That way if someone uses gameinfo.txt, it would remain operational. But if it's not present then key-values read from liblist.gam would remain effective and uneclipsed. And if liblist.gam is edited, new unique pairs would propagate to take effect, unless gameinfo.txt overrides them again. It seems like this would be least damaging change to me. It also allows for customization between engines.

Another thing if we started talking about gameinfo.txt: animated_title should play media\logo.avi if set, right? Because I can't get animated logo playing no matter what. Goes without saying, plays OK in original engine.

a1batross commented 11 hours ago

Yeah, I know, Xash should've just extended liblist.gam for it's own needs but gameinfo.txt is already used by Xash-specific mods. We are not breaking them.

I thought of a simpler behavior that if gameinfo.txt doesn't exist, then we directly load liblist.gam. If it exists, liblist.gam is always ignored.

animated_title doesn't make menu play logo.avi, it only plays logo.avi when WON-style background is found, if mod has Steam styled backgrounds (defined by resource/BackgroundLayout.txt file), then it won't apply it. The reason for this is that logo.avi doesn't have an alpha channel, it's expected to be used only with WON-style background it was made for. Also, logo.avi uses some very outdated codecs and Windows version of the engine expects them supported in the VFW. It's a legacy logic that I tried to rewrite several times utilizing ffmpeg but never succeeded.

animated_title instead hints the menu to look for Half-Life 25-th anniversary update files that simulate classic logo.avi animation with a bunch of TGA files.