[RFC] Fixing early design flaws (package format, package version]

Request new features or present your ideas.
User avatar
illwieckz
Project Head
Posts: 718
Joined: Sat Aug 11, 2012 7:22 pm UTC
Location: France
Contact:

[RFC] Fixing early design flaws (package format, package version]

Post by illwieckz »

What's happening?

In all projects some early decisions that looked good enough appear annoying later, when more stuff is done.

One of these early mistakes was to keep the pk3 file extension when the Dæmon VFS switched from classic id Tech 3 pak format to the newly dependency-based format. Well, because of this great change, our package format is not pk3 anymore. It raises a major issue: having to deal between standard pk3 and non-standard pk3 (ours) with ugly workarounds everywhere. :frown:

When TimePath added a cvar to enable third-party dæmon-based games loading legacy pk3, I suggested to add some metadata to pakpath directory, this way the engine would know if all the pk3 in one directory are standard pk3, and all pk3 in another directory are enhanced pk3. Well, even if the idea was not bad, it was wrong, it was wrong because it's wrong to flag the directory for the format of the files it contains instead of flagging the files themselves. :cyclops:

The PR was named “Support loading pak files which don't follow naming convention”, in fact, it would have been named “Support loading legacy pak format”. :wink:

I'm working on merging the old code by Neumond to enable pk3dir support in NetRadiant and GtkRadiant. The NetRadiant implementation is very complete, it can handles both standard pk3 and enhanced pk3, by the way, the code will probably never been merged as-is since it uses such an ugly workaround, it does that:

Code: Select all

# pseudo-gamepack:
pak_format = pk3

Code: Select all

# pseudo-code:
if pak_format == "pk3":
  if game == "daemon":
    findPk3WithDeps()
  else:
    findPk3()

:bugeyes:
That's bad , it means one day the NetRadiant code will look like that:

Code: Select all

# pseudo-code:
if pak_format == "pk3":
  if game in [ "daemon", "unrealarena", "xonotic" ]:
    findPk3WithDeps()
  else:
    findPk3()

You'll lose all the benefit of loadable gamepack, welcome back to GtkRadiant where you must hardcode everything. :cyclop: One of my earlier idea was to add a variable in the gamepack to flag the vfs, something like:

Code: Select all

# pseudo-gamepack:
pak_format = pk3
vfs_format = daemon

Code: Select all

# pseudo-code:
if pak_format == "pk3":
  if vfs_format == "daemon":
    findPk3WithDeps()
  else:
    findPk3()

This idea will work, but will still be wrong, the same way the idea to flag the directory was wrong. Every third-party program that have to deal with our paks and pakdirs will have to reproduces so many ugly workaround if they want to offer enhanced pk3 support: the Radiant editors, the Q3map2 compiler (currently loads every pk3dir so you don't know if your build is safe), the build tool I'm writing (Urcheon), and probably more. :frown:

We have to face the reality: our format is not PK3.

Let's introduce DPK.

Why not naming our package DPK? something like "Dæmon PacK" or "Deps PacK", also I thought it's a good idea to not increment the number (like PK5 or PK6) since other will have the same idea and the point is to not have to deal with different format with same extension. This way editors, compilers, builders, and even the game engine must be able to do:

Code: Select all

# pseudo-gamepack:
pak_format = dpk

Code: Select all

# pseudo-code:
if pak_format == "pk3":
    findPk3()
elif pak_format == "dpk":
    findDpk()

Clean, neat, awesome. :cool:

Also, an existing game full of legacy files (like Xonotic) switching to Dæmon engine would be able to do that:

Code: Select all

# pseudo-gamepack:
pak_format = [ dpk, pk3 ]

Currently in Daemon, enabling fs_legacypaks will load all versioned and non-versioned pak. If we switch to the DPK format, we will become able to load all legacy non-versioned PK3 but loads DPK the versioned way. We can also defines a priority: loads DPK first, PK3 later. This way, Xonotic Guys can switch to versioned package format for their own official packages, without having to load official packages they don't need, but being able to load legacy stuff, without having to fear them to overwrite their own stuff.

The first easy step would just be to add DPK as an extension name alternative first.
Adding DPK support in Dæmon ? Just add DPK extension as recognised one with PK3 first, then later you will be able to implement the format precedence.
Adding DPK support in GtkRadiant/NetRadiant/DarkRadiant ? just add the DPK extension as a recognized one alongside the PK3/PK4 one that are already there. Then later you can implement the package versionning for DPK.
Adding DPK support in XQF (server browser that parses some maps data) ? just add the DPK extension as a recognized one alongside the PK3 one. You want later save loading time just looking for "map-_.dpkdir" ? You can implement it later ! You will be able to do it !
Adding DPK support in q3map2 ? just add the DPK extension as a recognized one alongside the PK3 one that is already there, and later, if you have time, implement versioning.

The very first step will be to get the complete chain loading the DPK, it's just a matter of “if pak_format in ["pk3"] or ["dpk"]” if we have no time to polish it. Later, when the complete chain support DPK, when people are able to create content and play games using DPK, we can implement the versioning in software the knows nothing about it, later. :wink:

Extra benefit: fix early versioning mistakes

There is things that are not good in current package versions: many of them starts with a letter instead of a number: a5, b2, etc. It means 1.0 is smaller than alpha 1. Well, alpha/beta is a kind of quality flag, not a version, by the way, if someone wants to put the quality flag in the version string, he can safely put at the ends of the version string: 0.1a (alpha), 2.9b (beta), 6.8 (stable). The current workaround for packages that were badly versionned with a version string starting with a later is to start with r: r1.1.0 for example, by luck, r1.1.0 is smaller that src, but one day one guy will name his package map-castle_v2.pk3 and will not understand why the changes he does to map-castle_src.pk3dir does not show up. Also, there is plenty of old legacy maps with _trem versions on the web. When we will start to advertise them, people will found them and will put them on their servers and it will be very bad. :confused:

If we switch to DPK format, Unvanquished will not load the bad versionned PK3! It means every one get the opportunity to re-versionnize their package correctly! There is not many community mappers out there, they are still reachable so they can rename their package.

I also suggest to disable the DPK loading in engine and tools if the version starts with a letter instead of a number, logging a message so people will understand what is happening, with one or two exception: the src special version in build tools (for map-castle_src.dpkdir full of map source and lossless textures loaded by radiant and q3map2) and the test special version in engine (for map-castle_test.dpkdir full of bsp and crn files the engine load when a mapper tests what is he mapping before packaging). :smile:

Since we talk about version, I really don't like the current date policy. Why "2017-03-24-1524" and not "20170324-1524"? having so many hyphens is boring and my eyes have an hard time to read it fastly, it's the only place in the world where datetime is written in 4 chunks, it's not very comfortable, and it looks too verbose for a version string. Using the "one hyphen" format would be more compact and makes more sense: this way hthe hyĥen splits the version between date and time, it as a meaning, also, the hyphen will split between decimal numbers and sexagesimal numbers, which makes sense strongly. With hyphens everywhere it makes no sense at all. If we go the DPK way, we can redecides this kind of stuff, since there is no one DPK package out there. :grin:

Last edited by illwieckz on Tue Mar 28, 2017 9:17 am UTC, edited 4 times in total.

This comment is licensed under cc ​​by 4 and antecedent.

User avatar
illwieckz
Project Head
Posts: 718
Joined: Sat Aug 11, 2012 7:22 pm UTC
Location: France
Contact:

Re: [RFC] Fixing early design flaws (package format, package version]

Post by illwieckz »

So, to summarize my wall of text, after some talk with @Amanieu on the development channel, my suggestion is:

  • change the extension from .pk3 to .dpk, keep .pk3 support as a "legacy" format

  • reject dpk version numbers that don't start with a number

  • allow dpkdir version to start with a letter, so mapper does not have to name his source directory map-castle_9ZZZZZZZZZZZ.dpkdir, but can do map-castle_src.dpkdir for example.

That would be enough

About “why not bumping the source dpkdir version number?”:

  1. it's not doable, texture repositories will become submodules, no one wants to play the "readd submodule dance".
  2. it's not needed, the build tool I'm writing is able to compute the release pak version from git references (tags…)

[Edit: I also suggest switching to date format YYYYMMDD-HHMM in version string because so many hyphens are boring]

This comment is licensed under cc ​​by 4 and antecedent.

User avatar
Ishq
Project Head
Posts: 1145
Joined: Tue Mar 06, 2012 8:32 pm UTC

Re: [RFC] Fixing early design flaws (package format, package version]

Post by Ishq »

This seems acceptable to me.

User avatar
illwieckz
Project Head
Posts: 718
Joined: Sat Aug 11, 2012 7:22 pm UTC
Location: France
Contact:

Re: [RFC] Fixing early design flaws (package format, package version]

Post by illwieckz »

This a branch that adds pk3dir/dpkdir support in netradiant (based on neumond's code): gitlab.com/illwieckz/netradiant:dpkdir.

I'm not a C++ master, I can't say if the code is optimal (and is probably not unless someone unravell the netradiant's spaghetti code first :cyclops: ), by the way it works. This code is far better than original one just because it does not hardcode "if game == unvanquished" stuff.

Editing the Unvanquished Vega map in dpkdir (only packages set in DEPS are loaded):
Image

Editing the Xonotic Drain map in pk3dir (all packages loaded):
Image

If your game has legacy pk3 package but you want to benefit from dpk support for newer ones, just add support for both formats in yourgamename.game this way:

Code: Select all

archivetypes="dpk pk3"

This way netradiant will load all pk3dir but only the dpkdir in DEPS of your map dpkdir !

I also modified q3map2 (that had already pk3dir support) to loads dpkdirs as if they were pk3dirs (no dependency mechanism), haven't tried to compile something yet, but well, the change is trivial (just doing if pk3dir or dpkdir).

This comment is licensed under cc ​​by 4 and antecedent.

User avatar
illwieckz
Project Head
Posts: 718
Joined: Sat Aug 11, 2012 7:22 pm UTC
Location: France
Contact:

Re: [RFC] Fixing early design flaws (package format, package version]

Post by illwieckz »

I had nothing to code to add basic (no deps) dpkdir support to DarkRadiant:

Image
I just wrote this line in “unvanquished.game” file:

Code: Select all

archivetypes="dpk pk3"

And DarkRadiant loaded both pk3dir and dpkdir from same directory. If I write “dpk” only, it loads only dpk/dpkdir of course.It looks like DarkRadiant just handle unknown archive formats as zip and automagically adds the *dir counterpart. :bugeyes:

This comment is licensed under cc ​​by 4 and antecedent.

User avatar
illwieckz
Project Head
Posts: 718
Joined: Sat Aug 11, 2012 7:22 pm UTC
Location: France
Contact:

Re: [RFC] Fixing early design flaws (package format, package version]

Post by illwieckz »

Here is a pull request to add dpk/dpkdir support in Dæmon engine.

It also fixes some bugs that were there because the engine was trying to handle two different behaviors for the same format, it's now far easier to handle two different behaviors for two different formats.

So we have now an unmerged complete chain with dpk/dpkdir support: map editor, map compiler, game engine, and all of them can load legacy pk3/pk3dir if some legacy stuff need it.

That was fast. :bugeyes:

This comment is licensed under cc ​​by 4 and antecedent.

User avatar
Viech
Project Head
Posts: 2139
Joined: Fri Aug 03, 2012 11:50 pm UTC
Location: Berlin

Re: [RFC] Fixing early design flaws (package format, package version]

Post by Viech »

I'm fine with changing the package extension to dpk. I agree that our packages are sufficiently different from pk3 to justify this change, given that the current same-name policy actually causes issues.

However, I'd like to maintain full debian version strings and debian version sorting. It's well documented, it's flexible, it's good and it works for us. Not to mention Darren would haunt us if we changed it. Don't invent a new version system for no other reason than personal taste. Debian standards are safe standards. (Take that from an Arch user. :wink:)

For the alpha/beta/release version flag issue, it is as simple as using r for the final release, that is a01 < … < a99 < b01 < … < b99 < r01 < r99. After that we would not even have to stop as debian versions should sort r99 < r100, but don't quote me on that. And to be fair, "Release 25" is more human-memorable than some 6.14 fractional number.

For 2017-03-24-1524 versus 20170324-1524 I do not feel strongly about it, though I can more easily read the former format and thus have decided on it when I "invented" these. It's fine that these strings are long because they are exclusively for in-development pk3s that are passed along internally, which should be verbose and easy to tell apart from any release pk3! I'd just stick to what we have, if only for consistency with older packages.

Responsible for: Arch Linux package & torrent distribution, Parpax (map), Chameleon (map texture editor), Sloth (material file generator), gameplay design & programming, artistic direction

User avatar
illwieckz
Project Head
Posts: 718
Joined: Sat Aug 11, 2012 7:22 pm UTC
Location: France
Contact:

Re: [RFC] Fixing early design flaws (package format, package version]

Post by illwieckz »

Viech wrote:

I'm fine with changing the package extension to dpk. I agree that our packages are sufficiently different from pk3 to justify this change, given that the current same-name policy actually causes issues.

:thumbup:

However, I'd like to maintain full debian version strings and debian version sorting. It's well documented, it's flexible, it's good and it works for us. Not to mention Darren would haunt us if we changed it. Don't invent a new version system for no other reason than personal taste. Debian standards are safe standards. (Take that from an Arch user. :wink:)

My proposition requires unmodified full debian version sorting because it relies on it heavily. :tongue:

The idea to require released pak version starting with a digit is similar with the already implemented rule saying the engine requires map pak names starts with a “map-” prefix and the engine already requires the map pak name reuse the file name of the bsp it contains, and these map pak requirement does not change or break the debian file naming rules. The “map-” prefix and naming requirements for map packages are there because we need the computer discovering by itself a given pak is a map pak without having to extract the archive and apply fuzzy heuristics on its content.

The "release version string must be prefixed with a digit" rule was thought the same way, and it's not a personnal taste but a need I discovered while doing experiments with assets management and trying to solve common problems during the past two years.

These are the needs I discovered:

  • proper build chain needs the full pak/pakdir management including file naming to be doable by a tool: just type make and you get a pak. The tool must be able to name the pak by itself, including version string: it's a machine job, not a human job. To get reproduceable build, version must be predictable.

  • mappers need to be sure the engine is not loading previously released stuff over the in-development assets they are testing, therefore the engine must give them ways to prevent mistakes without having to use the "different homepath" trick to exclude old packages.

These two needs can be easily contented thanks to the standard debian sorting mechanism we use.

Enforcing the first number in pak verstion string to be a digit will allow mappers to rely on debian sorting to be sure they are testing their work in progress: they just have to use for their work in progress pakdir a version number that is always greater than the one allowed for release pak, greater the debian sorting way, and debian sorting says letter are greater than digits. The proposed solution does not break debian sorting, this is a solution based on debian sorting. The same way we prefix map- packages, we can say pak version must warrant to always be lesser than a pakdir according to debian sorting, the "first pak version number must be a digit" rule was thought to give this warranty thanks to debian sorting.

This comment is licensed under cc ​​by 4 and antecedent.

User avatar
Viech
Project Head
Posts: 2139
Joined: Fri Aug 03, 2012 11:50 pm UTC
Location: Berlin

Re: [RFC] Fixing early design flaws (package format, package version]

Post by Viech »

I don't see a problem here, or maybe I don't understand what you are saying. Can you try a more concise argument?

Random thoughts that may be relevant:

  • Developers can always use t for testing, it's greater than r for release. Personally I use source as my only in-development version string.

  • I think you can force dæmon to load a specific version of a given package via command line. You could then use this in case of doubt.

  • If you are hinting at enforcing different version strings for .pk3 and .pk3dir: I think these two package types should stay technically the same, except from one of them being zip-ed. (Even if this makes life harder for developers who don't understand that they are technically the same. They need to understand it anyway.)

  • Don't over-engineer the system just to prevent developers from making mistakes. If necessary educate them, or just leave it is a trap for people who'd previously develope a zzzzzzzzz-load-me-last mentality. :tongue:

  • The map prefix is not enforced for stylistic reasons alone. It allows the engine to list maps easily and to load map preview images from all available maps without loading non-map pk3s unnecessarily.

Responsible for: Arch Linux package & torrent distribution, Parpax (map), Chameleon (map texture editor), Sloth (material file generator), gameplay design & programming, artistic direction

User avatar
illwieckz
Project Head
Posts: 718
Joined: Sat Aug 11, 2012 7:22 pm UTC
Location: France
Contact:

Re: [RFC] Fixing early design flaws (package format, package version]

Post by illwieckz »

@Viech Google Chrome broke debian version sorting:
https://bugs.chromium.org/p/chromium/is ... 479419#c10

<`Ishq> I recommend we avoid using tildes in our pk3s for now.

It's a security issue.

This comment is licensed under cc ​​by 4 and antecedent.

Post Reply