Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic pk3dir and <pak>dir support #2554

Open
illwieckz opened this issue Jan 31, 2019 · 9 comments
Open

Automatic pk3dir and <pak>dir support #2554

illwieckz opened this issue Jan 31, 2019 · 9 comments
Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Enhancement New features

Comments

@illwieckz
Copy link

illwieckz commented Jan 31, 2019

A pk3dir is the directory equivalent of a pk3: extract pak0.pk3 content to a directory named pak0.pk3dir and tadaa, you got a pk3dir. It's very convenient for modding as everything can be put in pk3-like layouts while using fully editables directories. This feature is already supported by GtkRadiant, NetRadiant and DarkRadiant (and perhaps some other radiants) and some game engines so it's a must have for TrenchBroom.

I think the best implementation would be to do it the DarkRadiant way: for each existing package <extension>, support for <extension>dir would be implied. For example if someone adds this in a game config file:

        "packageformat": { "extensions": [ "pk3", "pk4", "dpk" ], "format": "zip" }

TrenchBroom would automatically enable support for pk3dir, pk4dir and dpkdir¹, and I'm sure people would appreciate waddir for same purpose.

_____ 
¹ pk4 is the doom3/quake4 package format which is just a pk3 with a 4, and dpk is an evolution of pk3 by the Dæmon engine (see more) that is thought to be loadable by existing pk3 code.

@kduske kduske added Area:Game Support Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Enhancement New features labels Jan 31, 2019
@kduske kduske added this to the 2019.2 milestone Jan 31, 2019
@kduske
Copy link
Collaborator

kduske commented Jan 31, 2019

A couple of comments and questions. First of, I think this should be optional, so the packageformat would be extended to

"packageformat": { "extensions": [...], "format": "zip", "packagedir": "${packagefile}dir"  }

The packagedir directive specifies the name pattern, which in this case is the package filename followed by dir, which resolves to your proposed pak0.pk3dir. If the packagedir directive is missing, then package directories will be disabled. I'm still unsure whether to enable this by default in the shipped game configurations.

Then I'm wondering about the behavior when both the package file and the directory are present, e.g. you have pak0.pk3 and pak0.pk3dir. The following possibilities exist:

  • ignore package file entirely
  • ignore package directory entirely
  • consider both the file and the directory, but
    • package file has higher priority in case of name conflicts or
    • package directory has higher priority in case of name conflicts

Could you find out how other editors handle this?

@ericwa
Copy link
Collaborator

ericwa commented Feb 1, 2019

Maybe it would be cleaner to allow multiple package formats like:

"packageformat": [ { "extensions": ["pk3", "pk4"], "format": "zip" },
                   { "extensions": ["pak"], "format": "idpak" },
                   { "extensions": ["pk3dir"], "format": "packagedir" } ]

and this gives you well defined priority as well. (There are engines e.g. FTEQW that support all 3 package types as well as others).

So we don't break all existing game configs, "packageformat" being a dictionary should be still supported.

From checking FTEQW sources it seems to load all of the files for a given package format, then all of the files for the next format, etc., but I'm not sure whether pk3dir comes before or after pk3.


Side node, we could express Quake's counterintuitive behaviour where loose files have a lower priority than files with the same name in pak's, which is currently hardcoded in TB, like this:

"packageformat": [ { "extensions": ["pak"], "format": "idpak" },
                   { "extensions": [], "format": "loosefiles" } ]

Quake 3 doesn't even load loose files not in a pk3 does it?
Unlike my other suggestion, this one would be a breaking change to the game config though.

@kduske
Copy link
Collaborator

kduske commented Feb 1, 2019

This looks like a good idea except if a package dir completly replaces its corresponding package, that is, if you find pak0.pk3 and pak0.pkdir, pak0.pk3 is not even loaded at all. @illwieckz could you please clarify this?

Otherwise I like your proposal, however the loosefiles entry should go before the pak entry to achieve Quake's behavior, since the priority is in ascending order.

@kduske kduske removed this from the 2019.2 milestone Feb 2, 2019
@illwieckz
Copy link
Author

illwieckz commented Feb 22, 2019

For what I know, in case of pk3dir and pk3 having the same base name, the pk3dir is to-be-released hence more recent than already released pk3. Because of that both content must be loaded because to-be-released stuff may extend already released stuff. Because of that too, in case of files having the same name within pk3dir and pk3, the file from pk3dir must overrides the file from pk3 because to-be-released work is expected to override previously released work.

I tried to verifiy reading the code but that was boring. So I made a test bed you can download there. It's a test map lying in a test mod for quake3 (use the fs_game variable as test in both radiant, q3map2 and quake3).

This contains a pk3dir, a pk3 and a map, both having a test basename.

The pk3dir contains 3 textures, one red named both, one blue named pk3dir, one gray named common.

reference

The pk3 contains 3 textures, one green named both, one yellow named pk3, and the same gray one named common.

Both both textures (either red or green) are light emitting shaders, that's useful to test q3map2 map compiler.

reference

Once loaded in radiant texture browser (here NetRadiant), the red one overrides the green one as expected and both blue and yellow are displayed as expected:

reference

This is verified in both NetRadiant:

reference

And in GtkRadiant:

reference

All radiants are GtkRadiant forks so if they don't behave the same, it's a bug. Technically radiant editors load pk3dir as yet another vfs (default vfs being the engine one and the home user one).

To test how q3map2 behaves, I turn red and green both textures into light emitting shaders and I wrote a compiled a map that relies on the both texture (there is a Makefile provided for reproduction purpose). This means q3map2 will paint lightmap with the average colors of the texture that overides the other one, it means the produced lightmap must be painted with a lot of red. This is true:

reference

This is how it is seen within engine, we clearly see that the red texture from the pk3dir was clearly used by q3map2 instead of the green one from the pk3:

reference

@illwieckz
Copy link
Author

illwieckz commented Feb 22, 2019

I did the same to test loose file, you can download extended test bed there

I added loose files, the both one being white and a loose one being black:

reference

We see that the loose both file take precedence in any way, in NetRadiant and GtkRadiant:

reference

reference

Don't worry if this one looks darker, you must notice that the vertical grooves are lighter than the grey frames:

reference

By q3map2 map compiler:

reference

By ioquake3 engine:

reference

So, loose file > file in pk3dir > file in pk3.

It does not sound counter-intuitive to me since loose files may be considered as to-be-released files.

@illwieckz
Copy link
Author

Since pk3dir is a vfs like the baseq3 directory is, the rule looks to be simple:

  • vfs extends (different content file name) or overrides (same content file name) pk3
  • vfs extends (different content file name) or overrides (same content file name) nested vfs (pk3dir)

This is verified by experience with reference editors (GtkRadiant, NetRadiant), reference map compiler (q3map2), reference engine (ioquake3).

@kduske
Copy link
Collaborator

kduske commented Feb 22, 2019

@illwieckz thank you very much!

@illwieckz
Copy link
Author

illwieckz commented Feb 22, 2019

@ericwa wrote:
Maybe it would be cleaner to allow multiple package formats like:

 "packageformat": [ { "extensions": ["pk3", "pk4"], "format": "zip" },
                    { "extensions": ["pak"], "format": "idpak" },
                    { "extensions": ["pk3dir"], "format": "packagedir" } ]

That's a side note but by reading how other things are done, the key may be named packageformats with a s.

I know it's possible to do a trick like « if list load as content else load as [ content ] » to avoid the packageformat|s thing (example in another project) but it looks like @kduske already went for something|s for other keys.

In any way I'm with you about the need to support multiple package formats.

@kduske wrote:
A couple of comments and questions. First of, I think this should be optional, so the packageformat would be extended to

"packageformat": { "extensions": [...], "format": "zip", "packagedir": "${packagefile}dir"  }

Why not but it looks over-engineered, other tools like radiant editors just look for [file ext]dir, this is the kind of situation that can be solved by agreeing it's standard than with code. And if we still want to keep it optional to avoid the unexpected use case were there would be really two extensions in this world conflicting with this rule (like an imaginary .bi file extension and an imaginary .bidir directory extension that would really not be an archive version of the directory version), we would just need a boolean like that:

"packageformat": { "extensions": [ "pk3", "pk4", "dpk" ], "format": "zip", "autodir": "true" }

Also that autodir (or any other key name you prefer) may be implicit true, people may be able to set it to false in the very rare case that may probably never happens.

Note that pak users and wad users may be very interested in.

So, there is an example of how it may look, with an implicit false:

Note: I added an imaginary p4k format that is zip too but must be not have a p4kdir alternative

"packageformats": [ { "extensions": ["pk3", "pk4"], "format": "zip", autodir="true" },
                    { "extensions": ["p4k"], "format": "zip" } ]
                    { "extensions": ["pak"], "format": "idpak" } ]

Now with an implicit true:

"packageformats": [ { "extensions": ["pk3", "pk4"], "format": "zip" },
                    { "extensions": ["p4k"], "format": "zip", autodir="false" } ]
                    { "extensions": ["pak"], "format": "idpak", autodir="false" } ]

To me the implicit true looks better since the pakdir idea is just convenient for everyone whatever the package format. Once you tasted it you'll never come back to something else. This is very good and it's one of the greatest idea if not the greatest idea in 20 years of mapping.

@hemebond
Copy link
Contributor

Could PhysicsFS be used to support this kind of feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Enhancement New features
Projects
None yet
Development

No branches or pull requests

4 participants