godot icon indicating copy to clipboard operation
godot copied to clipboard

Improve PCK loading filename handling

Open aaronfranke opened this issue 3 years ago • 4 comments

This PR changes the code for loading a PCK file, specifically how it finds the .pck file. Discussed on RocketChat.

The existing behavior of having mygame.exe and mygame.pck still works fine. The code in this PR is actually simpler while still having the functionality of the current master and more, although there are more lines of code because I abstracted it into a separate method and added more comments.

The code continues stripping the file name of extensions until it finds a PCK, or until there are no more extensions to strip. For example, let's say our executable is "mygame.x86_64.exe". First we look for "mygame.x86_64.exe.pck". If that fails, we look for "mygame.x86_64.pck". If that fails, we look for "mygame.pck". This allows having multiple executables for a game that share a specific PCK file, or providing different PCK files for different architectures or other custom situations. This PR also moves this logic into its own method.

Aside from allowing stripping more extensions, this PR does change the order of how things are searched, which changes behavior, but only in a minor way, and in my opinion the new behavior is much better. It will now search for the fullest names first in all locations before trying a shorter name. For example, let's say we have an executable named mygame.exe, in the current master it will search for .pck files in this order:

mygame.pck (in executable directory)
mygame.exe.pck (in executable directory)
mygame.pck (in current directory)
mygame.exe.pck (in current directory)
(even if there were more extensions to strip, the current master would stop after looking at these 4)

In this PR, the order will instead look like this:

mygame.exe.pck (in executable directory)
mygame.exe.pck (in current directory)
mygame.pck (in executable directory)
mygame.pck (in current directory)
(if there were more extensions to strip, this PR would continue)

I have already done some testing, and it works exactly as expected.

aaronfranke avatar Mar 25 '22 20:03 aaronfranke

I don't like this, it adds unnecessary flexibility and makes it harder to actually detect Godot games for something like SteamDB. I don't see a good rationale for adding this flexibility.

We should not encourage filenames with multiple "extensions", that's confusing.

akien-mga avatar Mar 25 '22 21:03 akien-mga

@akien-mga If users want to ship multiple executables for different architectures, how do you recommend this be done? On Linux there can be game.x86_64, game.x86_32, game.arm64 etc, since the extension is just a convention and we can just have a single extension for the architecture. On Windows the .exe extension is required so the best that could be done is something like game.x86_64.exe etc, or maybe game_x86_64.exe which would have one extension but then we can't reuse the same .pck file.

The flexibility is optional, we don't have to encourage its use or even document it. If all someone does is use the same names as in the current master and in 3.x, like game.exe, it will still work fine.

Aside from being a separate method, the complexity of this code is arguably simpler, since it doesn't have to repeat _load_resource_pack twice on each line for both names (with + without exension).

aaronfranke avatar Mar 25 '22 21:03 aaronfranke

Or ever do it in opposite direction, something like:

  • get the list of PCK in the executable folder first.
  • use exec_filename.begins_with(pck_file.get_file()) to determine which to load.

I'm not sure what the point of that would be. This suggestion seems to add complexity for no benefit. We shouldn't need to look at any other files in the folder. In my opinion, if we want to have the architecture somewhere in the executable name, we should just have it dot separated like is already the case with file extensions on Linux exports. Or, users could just not have the architecture in the name, and then it would work fine for existing uses.

aaronfranke avatar Mar 30 '22 07:03 aaronfranke

Yeah, this would be nice. Alternatives I see are not very attractive:

  • ship separate packages for each architecture or bundle x86 based versions together (they compress well) and arm together? It's definitely more complicated
  • ship multiple copies of the .pck file... in ZIP it might deflate well, but will still take space on user's disk
  • ship a hard/symbolic link "alias" to the .pck file so other executables will find it - but not all filesystems support this...
  • ship launch script that runs the extra executables with parameters to make them find the .pck file

Being able to have more options to have Godot runtime find the .pck file would be really useful...

unfa avatar Sep 12 '24 23:09 unfa