ec- / Quake3e

Improved Quake III Arena engine
GNU General Public License v2.0
1.21k stars 154 forks source link

Feature request: New filesystem #12

Open Chomenor opened 6 years ago

Chomenor commented 6 years ago

I've been working on this project for several years now. I've been trying to propose this for ioquake3, but so far it doesn't seem to have gotten much interest, so I thought I'd try here.

Here's a general overview of the project. You can also check the readme and design doc, if you haven't seen them already.

ID Pak Precedence

In this filesystem the ID paks have precedence over all other pk3s in baseq3, compared to the original behavior where pk3s alphabetically higher than 'p' override them. Permanent mod pk3s can still be used, but need to be moved to the 'basemod' folder instead of baseq3.

This change makes the game more stable against unwanted overrides from arbitrary pk3s, including pk3s downloaded from servers. It gives the user more control because only the pk3s they specifically select by placing them in basemod (or that are part of an fs_game mod) have override privileges.

Other Precedence Changes

Certain file precedence behavior has been adjusted to better handle conflict situations and respect and the intentions of the ID pak precedence policy. These changes have little to no practical compatibility impact in normal usage, but do prevent bugs caused by badly designed pk3s. The following examples show the main precedence changes and their motivation.

Scenario: The shader "console" is defined in gfx.shader in pak0.pk3. Suppose "ql_mapconversion.pk3" defines its own "console" shader in "shadersfromquakelive.shader". Due to quirks in the standard Q3 engine, shader precedence (for uniquely named .shader files) is the opposite of normal filesystem precedence. Therefore the shaders in pak0.pk3 do not get overridden and this map conversion 'works' because q > p. However, if somebody decides to rename the map pk3 file to "mapconversion.pk3", or the user's ID pak was renamed to "zpak0.pk3", or similarly prioritized by the engine, incompatible shaders would probably be loaded and put the game in a broken state.

Solution: Shaders follow the same conventions as other assets regardless of the name of the .shader file. This ensures the ID pak, mod/basemod precedence, and similar policies work as intended instead of potentially having the opposite effect. Due to the ID pak precedence policy, maps with excess shaders such as the one described continue to work without risk of destabilizing the game.

Scenario: A map pk3 contains certain shaders/textures. It is a popular map, and other maps copy assets from this map. An amateur map author decides they want to modify one of the assets in their map, but they leave the same name, causing the change to bleed into the original map as well when their pk3 is present.

Solution: The current map pk3 has precedence over other arbitrary pk3s in baseq3. This prevents a lot of conflict headaches for map authors, and even allows some 'mod' maps that modify assets without changing the map bsp to functionally coexist with the original map (e.g. padshop/adultshop). This behavior applies only to regular maps in baseq3, not maps from an ID pak or mod pk3.

Scenario: The shader "textures/base_trim/basemetalsupport" is not explicitly defined in a shader file, but it can be used by maps to reference the jpg image file of that name in pak0.pk3. Suppose a map author decides to put an explicit shader for this image in their pk3 to do some special effect. In the standard filesystem, the explicit shader will always override the image and the change will bleed into all maps.

Solution: Shaders continue to override images in most cases, because there are valid use cases for this. However, images in the ID paks or current map are special cases and can be protected from being overridden by external shaders.

Scenario: If a map pk3 includes "textures/base_trim/basemetalsupport" as a tga file, it would override the version in pak0.pk3 in spite of pk3 name or the ID pak policy because the renderer traditionally searches for tgas ahead of jpgs.

Solution: Precedence is applied regardless of image type, so a jpg in an ID pak or mod can override a tga in baseq3. This ensures filesystem precedence policies work as intended when multiple image types are involved. Similar behavior applies to sound file types as well.

Performance Improvements

This filesystem is designed from the ground up for high performance. There are several load time optimizations, including caching pk3 index data (matched by size and timestamp) to a file for faster startup, and avoiding the need to reindex the filesystem between maps for faster map load times.

Here are some benchmarks comparing my ioquake3 filesystem port versus current quake3e with different numbers of map pk3s installed in baseq3.

Test methodology:

Results:

configuration cold start warm start q3dm17(1) q3dm17(2) q3dm7(1) q3dm7(2)
clean ioq3 302 ms 196 ms 581 ms 296 ms 535 ms 462 ms
clean q3e 378 ms 202 ms 559 ms 457 ms 639 ms 601 ms
250pk ioq3 1286 ms 213 ms 595 ms 310 ms 555 ms 480 ms
250pk q3e 1536 ms 363 ms 752 ms 632 ms 833 ms 778 ms
500pk ioq3 2177 ms 241 ms 601 ms 320 ms 545 ms 490 ms
500pk q3e 2804 ms 538 ms 985 ms 876 ms 1120 ms 1053 ms
1000pk ioq3 2220 ms 239 ms 609 ms 331 ms 570 ms 486 ms
1000pk q3e 5049 ms 838 ms 1333 ms 1185 ms 1469 ms 1397 ms
4000pk ioq3 2054 ms 280 ms 637 ms 350 ms 583 ms 515 ms
4000pk q3e 19586 ms 4201 ms 6580 ms 5257 ms 5010 ms 4900 ms

Notes:

Other features

Functional features:

Implementation features:

Conclusion

I'm proposing this filesystem as a possible candidate for Quake3e because it seems to fit with existing Quake3e design priorities; i.e. an emphasis on practical Quake 3 multiplayer, map support, and performance improvements.

If you would like, I'm willing to assist in porting my filesystem to Quake3e, including helping with support and technical issues and perhaps making a prototype integration. Based on a brief review of the filesystem code in Quake3e I think the port should be straightforward.

ec- commented 6 years ago

Thank you for your work and intentions. I'm already reversed shader load order (since 2009) and commented known buggy shaders in renderer code. For future - sure, I'd like to implement performance-related ideas, piece by piece and with intermediate releases to check for bugs. Rest of the ideas is more or less discussable, I'll leave them in mind.

For some operative feedback or any suggestion - feel free to join discord channel related to this engine https://discordapp.com/invite/X3Exs4C

Chomenor commented 6 years ago

Normalizing the shader order is good. The original reversed behavior is pretty ridiculous :/ My comment is that prioritizing ID paks can be helpful to prevent breakage from bad map conversions that were only tested under the reversed order.

There are actually ways to integrate my filesystem with some intermediate steps, if you are concerned about bugs. A large part does need to be integrated at once, but some features can be initially disabled, and some changes outside the filesystem code can be deferred. For example the renderer can be left unchanged and keep using the old API doing its own shader parsing, and updated later to use the new filesystem image and shader lookup functions.

Generally my filesystem is well tested and very stable, and I think the architecture is a lot better in the long run than what can be done with incremental changes on the original code. If there are any specific bugs or concerns you have with implementation that make you feel uncomfortable integrating it, feel free to ask me about it any time.

ensiform commented 6 years ago

Is it fully intrgrateable with compatibility with vanilla servers etc?

Chomenor commented 6 years ago

It should be compatible with all servers. Part of my normal testing routine is to connect to every public Quake 3 server I can find, especially download enabled servers.

ensiform commented 6 years ago

Also, can it be made to be compatible with dll mods? Such as for Enemy Territory? They utilize extraction from mp_bin.pk3 to handle it normally.

Because if this does move forward in the mean time, I would want to port it to my fork of ET (ETe) which integrates most things from here for ET.

Chomenor commented 6 years ago

Yes, it shouldn't be hard to add such a function.

I'm not familiar with Enemy Territory at all. How are dll pk3s selected? Is it like Quake where the highest priority pk3 in the current fs_game is used, or are there other conditions? Does qvm support exist in Enemy Territory as well?

I'm also curious how the security model works if pk3s downloaded from servers can contain dlls? Normally you want to avoid trusting remote servers with full code execution privileges.

ensiform commented 6 years ago

In normal mods there is only a mp_bin.pk3 that is always overridden when an update happens. There is no security model. Though I am not sure it allows downloading of mp_bin off hand. It does prevent downloading of mp_bin in the base folder.

Chomenor commented 6 years ago

By overridden do you mean it gets replaced, or a differently named pk3 gets used instead? The download folder restriction system in my filesystem could help with security. It can block or restrict qvm files to only allow qvms on a trusted hash list. It could probably be adapted to handle dlls as well.

If you are interested in porting my filesystem, it is mostly a matter of transferring the source files and the ifdefs, but there are a few tricky areas that can require more work, as well as anything game-specific that is changed or added from ioquake3. If you would like to open an issue for it on your ETe repository we could discuss it more there.

ensiform commented 6 years ago

By overriden I mean, there is only ever a file called "mp_bin.pk3" AFAIK. So when a mod distributes a new version, they just override and update that file with new dll/so. I know that the etlegacy project diverged from this behavior but I am not sure of any other vanilla compatible mods that have.

ensiform commented 6 years ago

I will hold off on creating an issue there until there is progress here, because I just manually merge changes from this repository for the most part.