Unvanquished icon indicating copy to clipboard operation
Unvanquished copied to clipboard

Suggestion: create a basic `res-common` package to tell NetRadiant what are the basic packages to load

Open illwieckz opened this issue 3 years ago • 7 comments

The reason why unvanquished_src.dpkdir exists in UnvanquishedAssets/src is that NetRadiant needs it to edit maps.

NetRadiant needed this dpkdir to edit maps and q3map2 probably needed it as well for compiling maps because:

  • the unvanquished dpkdir was very large with plenty of files and then needed for mapping,
  • the VFS design inherited from Quake3 is implemented by both the Dæmon game engine, the NetRadiant level editor and the Q3map2 level editor.

Note: In contrary to some popular belief I have read, the current repository layout isn't decided by the Urcheon design. In the first place, both the repository layout and the Urcheon design had to fulfill the same requirements inherited from the Quake 3 VFS design. In the second place the Urcheon design was then constrained by this repository layout. Any tool dealing with more than one dpkdir and tools like q3map2 have those constraints, we don't have them as an option.

The bilocation problem, cause of submodules

The reason why unvanquished_src.dpkdir was turned into a submodule was a bilocation problem. At the same time this folder had to exist in UnvanquishedAssets/src, people expected this folder to also exist in Unvanquished/pkg for convenience.

While not being perfect, turning unvanquished_src.dpkdir into a submodule provided a solution for the bilocation problem.

Problems

The known shortcomings are:

Problem 1: split pull requests

Pull requests have to be split between Unvanquished and unvanquished_src.dpkdir, it makes contributing complicated.

Problem 2: two commit references for the same package

Since nexe commit references and unvanquished_src.dpkdir commit references are not the same, what commit to use as version string if not a tag?

There are two other problems not caused by the submodule itself, but we also need to fix them or to address them in some way and some solution may fix or make easier to fix those problems:

Problem 3: manual nexe merge into the dpk

Merging the nexe into the released dpk is adding manual a task to the release process.

Problem 4: Delta paks of nexe doesn't make sense.

A nexe always replace a nexe, it is not added alongside one. So delta paks of paks containing nexe are suboptimal.

Doing a delta pak of the unvanquished dpk is costly in file space because nexe are big, and at some point we start having more file size spent in delta paks with useless nexe than doing a non-delta package. Also the release validation script always complain that we provide more nexe than needed.

Solutions

Two solutions that keep unvanquished_src.dpkdir as a folder in UnvanquishedAssets/src have been proposed but never implemented. A third solution became possible with time.

Solution A: game code in unvanquished_src.dpkdir

This fixes 1 and 2.

Rename Unvanquished as unvanquished_src.dpkdir, storing the game code and the base assets in the same folder, that's what Xonotic does and it works well.

One shortcoming is that we actually add the engine as a submodule of the game code repository and then cloning recursively UnvanquishedAssets would also clone the engine. Xonotic stores the engine folder outside of the base package repository:

xonotic/ |→ data/ |→ xonotic-data.pk3dir (game code, base media files)
         |        |→ xonotic-music.pk3dir
         |        |→ <other>.pk3dir
         |→ darkplaces (engine)

Solution B: create a special dpk for the game code that is not the unvanquished dpk

This fixes 2, 3 and 4.

Rename unvanquished_src.dpkdir to res-unvanquished_src.dpkdir and ship game code in a dpk only shipping game code, for example named unvanquished and or mod-unvanquished and relying on res-unvanquished in DEPS.

That's what Tremulous did (except they did not have DEPS, and they did not have git data repositories):

tremulous/ |→ base/ |→ vms-1.1.0.pk3 (game code)
                    |→ data-1.1.0.pk3
                    |→ <other>.pk3

This layout removes the need to manually need to merge the nexe into the unvanquished dpk at release time, this also solves the problem about having different git references and then two versions for a single package, since there are two packages. We already have a tool able to produce a vm dpk only containing nexe from the Unvanquished repository. The shortcoming is that people still have to split their PR into multiple repositories.

Solution C: remove the need for NetRadiant to load unvanquished_src.dpkdir to edit maps

This fixes 1 and 2.

So, with time a third proposition started to become possible, the day NetRadiant doesn't need unvanquished_src.dpkdir to edit maps, the need for unvanquished_src.dpkdir to be stored in UnvanquishedAssets/src disappear and then the bilocation problem disappear, and with the bilocation problem disappearance, the need for a submodule disappears.

At first the unvanquished_src.dpkdir was very huge, but with time more and more things from there were moved to other dpkdir (like res-buidlables_src.dpkdir, res-players_src.dpkdir, res-weapons_src.dpkdir, etc.), sometime we even split too much as we had to reimport some files into unvanquished_src.dpkdir at some point.

The unvanquished_src.dpkdir package is now very small. I just got a look at it and unless I miss something, the only thing from it that is now needed by NetRadiant is the DEPS file. NetRadiant needs the DEPS file to load both tex-common and res-buildables by default (and maps should not rely on res-buildables). It looks like now all the other files from this folder (textures, materials…) are only used by the game, not the level editor neither the map compiler.

So, the idea is to create a very small package named res-common_src.dpkdir (or something like that) that just contains one DEPS listing res-buildables and tex-common. That sounds a bit over-engineered to make a dpk just for a DEPS file to configure NetRadiant, but if I haven't missed anything, this would be the only thing remaining to be able to remove the last remaining need for unvanquished_src.dpkdir to live in UnvanquishedAssets/src and then remove the need for a submodule.

This would not provide a solution for merging the nexe into the dpk, but by removing the submodule that would also solve the double commit reference issue for the same dpk.

On merging nexe in unvanquished dpk

…or merging unvanquished files in nexe dpk.

While I'm at it, I also answer to some popular belief I have read that to be able to produce a dpk containing both nexe game binaries and files from the unvanquished_src.dpkdir it would be required to solve the bilocation problem first.

It is already possible to implement a script or maybe a cmake target to both produce a dpk from nexe and files from unvanquished_src.dpkdir, and this has been true since the beginning.

The unvanquished_src.dpkdir was even designed on this purpose from the start, and the choice of formats used in it this repository was made on this purpose.

Solving the bilocation problem isn't required to implement this. We already have a script that creates a dpk from nexe, we can extend it to also zip the files from unvanquished_src.dpkdir. This isn't implemented yet for the only reason no one implemented it.

Edit: also, it may also be contrary to some popular belief, but I'm indeed not against that, I made the decision to make this possible at the very moment those repositories were created, I wrote the nexe packaging script, I even have a pull request to compute the versions like Urcheon if I'm right. Things just only go with small steps and slowly if I'm alone on this.

illwieckz avatar Sep 09 '22 05:09 illwieckz

Is checking out everything from UnvanquishedAssets really the normal way to do mapping? I've never heard about that. I thought they used released DPKs.

slipher avatar Sep 09 '22 05:09 slipher

Is checking out everything from UnvanquishedAssets really the normal way to do mapping? I've never heard about that. I thought they used released DPKs.

Mapping against released dpk is good when you do a community map, but as an Unvanquished developer you may want or need the latest repositories.

To answer another question from IRC:

you can only give radiant 1 dpk to load? why can't you tell it the 2 you want directly?

In fact radiant doesn't even have yet a way to set this dpk, so for now we workaround this by setting a DEPS that is not in a dpk, abusing the fact NetRadiant reads this file outside of dpkdir, not like Dæmon (so we are lucky the VFS only behaves to 95% the same).

So yes we can list more than one files in that NetRadiant DEPS without having a res-common package. The problem is that modifying that DEPS files used by NetRadiant requires a new radiant release, while shipping a new dpk with a DEPS file means there is no need to re-release radiant if we update the list. If we do a res-common or something like that, it is only required to edit that NetRadiant DEPS file once and then we have full control on the list on our side.

The DEPS is there: https://github.com/Unvanquished/unvanquished-mapeditor-support/blob/master/src/deps/DEPS

This is a repository owned and controlled by us, but NetRadiant fetches it at build time, so this is updated in NetRadiant release builds when NetRadiant is rebuilt.

If we decide to not do a dummy package and edit the NetRadiant DEPS it looks like we have a solution to the problem anyway, we just have less control on it.

What I find interesting is that it looks like we can now remove the need for the unvanquished dpkdir, whatever ugliness we prefer.

illwieckz avatar Sep 09 '22 05:09 illwieckz

Sounds good. How about naming the package res-leveleditor, since it's a hack existing only for Radiant?

slipher avatar Sep 09 '22 05:09 slipher

Yes, whatever the name.

illwieckz avatar Sep 09 '22 05:09 illwieckz

That just makes me think we may even be able to ship some other files like the .def files (entity definitions), so having a special leveleditor package may be a good idea for other problems too. I have to check the support from editors though. This is a side note on the topic because entities definitions are unrelated to the sumodule problem, but if that leveleditor package also solves other problems, this weights more in favor of it.

Something I have not said is that solution C can be implemented at the same time as A or B. Solution C just means the submodule bilocation problem can be fixed without implementing solution A or B.

illwieckz avatar Sep 09 '22 06:09 illwieckz

If i understand it right the reason to have unvanquished_src.dpkdir a separate repository is to pull and there are no pushes to it happening?

Would it be possible to just merge unvanquished_src.dpkdir into Unvanquished but also keep the repository as a mirror of the Unvanquished subfolder and update it automatically?

Gireen avatar Sep 09 '22 09:09 Gireen

Would it be possible to just merge unvanquished_src.dpkdir into Unvanquished but also keep the repository as a mirror of the Unvanquished subfolder and update it automatically?

Doing it one-way would be possible but manual (possible to make a script, but not to just use the e.g. mirror functionality on gitlab). But I think it would just be confusing for people modifying the repo then realizing (eg) that they can't do a PR.

necessarily-equal avatar Sep 10 '22 00:09 necessarily-equal

I created the res-leveleditor_src.dpkdir repository:

  • https://github.com/UnvanquishedAssets/res-leveleditor_src.dpkdir

It contains the DEPS file required by level editors, which is the last remaining file we needed from unvanquished_src.dpkdir requiring unvanquished_src.dpkdir to be a submodule of UnvanquishedAssets.

The DEPS file allows to edit the list of packages required by the editor by releasing the game instead of releasing the editor (so we have full control of the release schedule on this part).

We we may even ship the entity files in that repository so we also update them when releasing the game instead of releasing the editor (yet again, giving us full control on such release schedule). Edit: Right now netradiant doesn't load entity files from packages.

illwieckz avatar Jan 26 '23 15:01 illwieckz

What happens now that unvanquished_src.dpkdir is not a standalone repository?

slipher avatar Nov 29 '23 05:11 slipher

Gentle bump, is this still an issue? Or what changed?

On 29 November 2023 06:51:49 CET, slipher @.***> wrote:

What happens now that unvanquished_src.dpkdir is not a standalone repository?

-- Reply to this email directly or view it on GitHub: https://github.com/Unvanquished/Unvanquished/issues/2186#issuecomment-1831263793 You are receiving this because you commented.

Message ID: @.***>

necessarily-equal avatar Jan 13 '24 21:01 necessarily-equal

Done.

illwieckz avatar Jan 13 '24 22:01 illwieckz