View Issue Details

IDProjectCategoryView StatusLast Update
0005711DarkRadiantSaving and loadingpublic05.09.2021 13:57
Reporterillwieckz Assigned Togreebo  
PrioritynormalSeverityfeatureReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.13.0 
Fixed in Version2.14.0 
Summary0005711: Change Quake3 map exporter to write "legacy" brush syntax
DescriptionWhen opening a map using the Q3 Legacy brush format, it is saved using the non-Legacy Q3 format. It looks like there is no writer for this format, only a parser.

The following instructions to reproduce will produce a diff like that:

```
-( 384 16 304 ) ( -384 48 304 ) ( -384 16 304 ) shared_vega/trim03a 64 0 90 0.125 0.125 134217728 0 0
-( 384 48 304 ) ( -384 48 320 ) ( -384 48 304 ) common/caulk 0 0 0 0.5 0.5 134217728 0 0
-( 384 48 320 ) ( -384 16 320 ) ( -384 48 320 ) common/caulk 0 0 0 0.5 0.5 134217728 0 0
-( 384 16 320 ) ( -384 16 304 ) ( -384 16 320 ) common/caulk 0 0 0 0.5 0.5 134217728 0 0
-( -384 16 320 ) ( -384 48 304 ) ( -384 48 320 ) common/caulk 0 0 0 0.5 0.5 134217728 0 0
-( -96 56 -64 ) ( -96 8 -64 ) ( -96 56 64 ) common/caulk 0 0 0 0.5 0.5 134217728 0 0
+brushDef
+{
+( -96 48 304 ) ( -384 16 304 ) ( -96 16 304 ) ( ( 3.827021247e-18 0.0625 0.5 ) ( 0.0078125 -4.783776559e-19 0 ) ) shared_vega/trim03a 134217728 0 0
+( -384 48 304 ) ( -96 48 320 ) ( -384 48 320 ) ( ( 0.03125 0 0 ) ( 0 -0.03125 0 ) ) common/caulk 134217728 0 0
+( -384 48 320 ) ( -96 16 320 ) ( -384 16 320 ) ( ( 0.03125 0 0 ) ( 0 -0.03125 0 ) ) common/caulk 134217728 0 0
+( -96 16 320 ) ( -384 16 304 ) ( -384 16 320 ) ( ( 0.03125 0 0 ) ( 0 -0.03125 0 ) ) common/caulk 134217728 0 0
+( -384 16 320 ) ( -384 48 304 ) ( -384 48 320 ) ( ( 0.03125 0 0 ) ( 0 -0.03125 0 ) ) common/caulk 134217728 0 0
+( -96 48 320 ) ( -96 16 304 ) ( -96 16 320 ) ( ( 0.03125 0 0 ) ( 0 -0.03125 0 ) ) common/caulk 134217728 0 0
+}
```

Note: the parser also suffers from a rotation bug so along the format difference, some values may be wrong because being wrong at read time, see https://bugs.thedarkmod.com/view.php?id=5710
Note: you can safely ignore the fact the source references does not have comments like `// brush 0`, we just strip them to reduce diff noise in map repository but writing them is correct.
Steps To Reproduce# Assuming the PROFILE environment variable is set
# to the DarkRadiant installation directory.

# Download the .game file.
mkdir games
cd games
wget https://raw.githubusercontent.com/Unvanquished/unvanquished-mapeditor-support/master/build/darkradiant/games/unvanquished.game
cp -a unvanquished.game "${PREFIX}/share/darkradiant/games/unvanquished.game"
cd ..

# Download some data sources.
mkdir pkg
cd pkg
git clone https://github.com/UnvanquishedAssets/tex-common_src.dpkdir.git
git clone https://github.com/UnvanquishedAssets/tex-vega_src.dpkdir.git
git clone https://github.com/UnvanquishedAssets/map-vega_src.dpkdir.git

# Download the definition file.
mkdir def
cd def
wget https://raw.githubusercontent.com/Unvanquished/unvanquished-mapeditor-support/master/build/darkradiant/pkg/def/entities.def
cd ../..

# Run DarkRadiant and configure it for Unvanquished game
# and set the engine path to the folder containing the pkg/ folder
# and fs_game to “pkg”.
"${PREFIX}/bin/darkradiant"

# Quit DarkRadiant and reopen it with a map source.
"${PREFIX}/bin/darkradiant" "$(pwd)/pkg/map-vega_src.dpkdir/maps/vega.map"

# Add a brush, delete it, save the map.

# Have a look at the diff.
( cd pkg/map-vega_src.dpkdir/maps ; git diff )
Additional InformationIf the testing environment for https://bugs.thedarkmod.com/view.php?id=5710 is already set up, the steps to reproduce are only:

```
# Open DarkRadiant with a map source.
"${PREFIX}/bin/darkradiant" "$(pwd)/pkg/map-vega_src.dpkdir/maps/vega.map"

# Add a brush, delete it, save the map.

# Have a look at the diff.
( cd pkg/map-vega_src.dpkdir/maps ; git diff )
```
TagsNo tags attached.

Relationships

related to 0005710 resolvedgreebo Q3 Legacy BrushDef parser sometime produce some wrong texture rotation 

Activities

greebo

greebo

13.08.2021 18:04

administrator   ~0014289

Re: >> It looks like there is no writer for this format, only a parser.

There is the "Quake 3" map format which should be available for writing Q3 maps. Can you see that option in the save dialog or is it broken somehow?
greebo

greebo

13.08.2021 18:07

administrator   ~0014290

Ah sorry, I read again, and I the map is indeed saved as Q3, but not using the what is called "legacy" format (without brushDef keywords).

So your request is to add an exporter for that format? Is any game (yours?) actually still using that format? And there I thought the Q3 format is old already, so nobody would want to write the one before that.
illwieckz

illwieckz

13.08.2021 18:42

updater   ~0014291

> So your request is to add an exporter for that format?

Yes

> Is any game (yours?) actually still using that format?

Yes, all our maps (Unvanquished, https://unvanquished.net/ ) are using that format.

Also some of our tools expect that format, for example: https://github.com/DaemonEngine/Chameleon

Note: NetRadiant selects the quake3 non-legacy brush format when the game uses the quake3 format and `brush_primit` is set to 1 in `<game>` element in .game file and the legacy brush format otherwise (`brush_primit` not being set or being 0).
greebo

greebo

13.08.2021 18:59

administrator   ~0014292

I see. It's not exactly matching the idTech4 focus that DarkRadiant is trying to deliver, but I guess it won't be too much effort to add a separate "Quake 3 (Legacy)" format which uses that older syntax.
illwieckz

illwieckz

14.08.2021 09:08

updater   ~0014293

Last edited: 14.08.2021 09:16

I looked at some NetRadiant gamepacks to see how much games use the legacy Q3 format and how much games use the non-legacy Q3 format, and the result is surprising:

# 1 gamepacks with legacy Q3 brush format (maptypes="mapq3" brushtypes="quake3")

## 1.1 distributed with NetRadiant (expected to be working)

darkplaces.game (Darkplaces based, Quake)
et.game (Wolfenstein: Ennemy Territory)
neverball.game (Neverball)
nexuiz.game (Nexuiz)
oa.game (OpenArena)
osirion.game (Osirion)
smokinguns.game (Smokin' Guns)
trem.game (Tremulous)
turtlearena.game (Turtle Arena)
unvanquished.game (Unvanquished)
warsow.game (Warsow)
wolf.game (Return to Castle Wolfenstein)
wop.game (World of Padman)
q3.game (Quake III Arena)
q1.game (Quake)
quakelive.game (Quake Live)
zeq2lite.game (ZEQ2 Lite))

## 1.2 not distributed with NetRadiant (from http://svn.icculus.org/gtkradiant-gamepacks/)

ja.game (Jedi Knight: Jedi Academy)
jk2.game (Jedi Knight 2: Jedi Outcast)
sof2.game (Soldier of Fortune 2: Double Helix)
stvef.game (Star Trek Voyager : Elite Force)

## 1.3 not distributed with NetRadiant, generated by GtkRadiant using synapse.config and <api name="map">mapq3</api> (expected to be working, using synapse.config with mapq3, GtkRadiant also support some other games already listed in NetRadiant list, so I keep this list short)

urt.game (UrbanTerror)

## 1.4 not distributed but generated by GtkRadiant using synapse.config and <api name="map">mapq3</api> (from http://svn.icculus.org/gtkradiant-gamepacks/)

reaction.game (Reaction)

# 2 gamepacks using non-legacy Q3 brush format (maptypes="mapq3" brushtypes="quake3" brush_primit="1")

## 2.1 distributed with NetRadiant (expected to be working)

xonotic.game (Xonotic)

# 3 gamepacks using non-legacy Q3 brush format but with unrecognized obsolete .game file syntax (maptypes="mapq3bp" brushtypes="quake3bp") without support in GtkRadiant neither NetRadiant (if it existed, it was removed before 2006 and the start of git history)

## 3.1 not distributed with NetRadiant, from http://svn.icculus.org/gtkradiant-gamepacks/

jabp.game (variant for ja.game)

Well, in a way, this does not surprise me: gamepacks lack a lot of care, what is rarer than a Radiant contributor is a gamepack contributor. :D

So all those gamepacks are just relying on configurations from the very first days and then mappers read and write the legacy Q3 brush format.

I don't know when the “Alternative Texture Projection” (non-legacy Q3) brush format was added, but the older commit in GtkRadiant history from 2006 already has it with brush_primit="1" syntax support (but did not have maptypes="mapq3bp" neither brushtypes="quake3bp" syntax support).

https://github.com/TTimo/GtkRadiant/commit/12b372f89ce109a4db9d510884fbe7d05af79870#diff-3168e181773635fd4b28bd065f201919f62743a16ac3156144208cbc3275d622R283-R285

GtkRadiant never enables the non-legacy format by default, instead it displays an option to enable it in project preferences saying “Use brush primitives in MAP files (NOTE: experimental feature, required by the texture tools plugin)”, and does not write the "brush_primit" key in the game file but in the "user0.proj" file, so from GtkRadiant point of view this is not a per-game option but a per-project option. Also it gives an hint the usage of that non-legacy format was motivated by a plugin, and I don't know what this plugin is for…

So it looks like only Xonotic is making use of the new format by default (and has a very active mapping scene).

That said:

- I may have missed some game packs update that never reached me
- some per-game Radiant may have switched on the new format
- the new format can be switched on and off in both GtkRadiant and NetRadiant when mapping for the same game
- some game may give instructions to mapper to switch to non-legacy format as first configuration step

But:

- all idTech 3 derivatives seem to all use the legacy brush Q3 format
- some non-id Tech 3 derivatives also use some id Tech 3 format and among them, only Xonotic made non-legacy Q3 format the default brush format

So the big picture is that there is in the wild maybe thousands of maps with the non-legacy brush format, and dozens of thousands maps with the brush legacy format.

Maybe at some point it may be useful to change the naming, because the one named “Legacy” in DarkRadiant is the default one outside of DarkRadiant, when the one not named Legacy in DarkRadiant is the one named ”Alternate” or ”Brush Primitive” outside of DarkRadiant.
greebo

greebo

14.08.2021 13:38

administrator   ~0014294

Sounds like moving to the "non-legacy" format would be entirely possible but game maintainers never cared to define it as the default.

Anyhow, I'll probably rename the brush format then too, seeing that it is much more widely used than the one using the texture projection. I'll follow the naming to use Quake 3 as what I called legacy before, and to use "Quake 3 with BP" as the newer one.
greebo

greebo

14.08.2021 13:38

administrator   ~0014295

At any rate, thanks for doing the leg work and checking out the game packs.
illwieckz

illwieckz

14.08.2021 16:16

updater   ~0014296

> Sounds like moving to the "non-legacy" format would be entirely possible but game maintainers never cared to define it as the default.

Yes that's the point. Game packs are lacking a log of care. Usually, once someone has a proof of concept, no one modify it for 15 years. :D

Projects like Xonotic are very active and have people taking the time to investigate the various options so I'm not surprised they did the switch.

> I'll follow the naming to use Quake 3 as what I called legacy before, and to use "Quake 3 with BP" as the newer one.

That sounds good though I'm not sure if “BP” is explicit enough. For example it took me some time to figure out why there was a “jabp.game” game pack, until I discover the file named `radiant/bp_dlg.cpp` in GtkRadiant source tree which is the dialog dealing with format mismatch (if brush primitives format is used or not).

What do you think would be better to select the default writing brush format in a DarkRadiant .game file? It looks like in NetRadiant the map writing format is set with the `type` key (while previously in NetRadiant this key usually contained the game name), and it looks like the maptypes, brushtypes and patchtypes keys are not used anymore in DarkRadiant.

NetRadiant sets the `type` key to the game name, and `maptypes` to mapq3, `patchtypes` to `brushtypes` to quake3 and `brush_primit` to 1 to set the quake3 map format with brush primitives, but given DarkRadiant uses the `type` key for the map format, with possible values being `doom3`, `quake4` or `quake3` if I'm right, maybe the “brush primitives quake 3 format” would be set with `type="quake3bp"`? We have a tool to produce game packs for various editors (https://github.com/Unvanquished/unvanquished-mapeditor-support/tree/master/mkeditorpacks), so I can adjust the DarkRadiant game file writing accordingly.
greebo

greebo

14.08.2021 19:56

administrator   ~0014297

> What do you think would be better to select the default writing brush format in a DarkRadiant .game file?

Indirectly, yes, through the format. The brushtype attribute itself is no longer in use, as far as I'm aware. DarkRadiant's goal has never been to support all those games like GtkRadiant or other flavours, we specialised on idTech4, and I prefer not having to re-implement any of that logic.

Right now, the way DR needs to be set up is to define a .game file for Unvanquished with game type "unvanquished" instead of "doom3", and DR would have a specialised Map Format for Unvanquished built in. This map format is named "unvanquished" and dictates how the brushes and patches need to be written. This approach makes it possible to save a map using a different format by selecting it in the "Save As" dialog, that flexibility would get lost if the Q3 brush exporter is always using the format defined in the currently open game - that's why I went down that road and decoupled the loaders from the .game file iirc.

Do you have a custom fork for DarkRadiant set up for Vanquished? Would a plugin work for you, or are you working with vanilla DR? I could help you set up the format classes, in case it's not obvious from the examples in DR (like the Quake 4 format).
greebo

greebo

14.08.2021 20:08

administrator   ~0014298

Last edited: 14.08.2021 20:13

Second option is to change the Q3 exporter (or add a dedicated one for the "legacy" brush format) and your game is using that one instead of a custom one. Unvanquished .game file refers to gametype "quake3", and the "quake3" map format writes the older brush syntax by default.
illwieckz

illwieckz

16.08.2021 16:03

updater   ~0014302

> Do you have a custom fork for DarkRadiant set up for Vanquished? Would a plugin work for you, or are you working with vanilla DR?

We don't have custom fork for DarkRadiant, and we don't need a plugin for Unvanquished as far as I know. We have a generator that generates a game pack for the various editors we care about. Basically for DarkRadiant it's a mater of producing a DarkRadiant .game file and an .def file in Doom3 format describing entities, both seems to now be working correctly with vanilla DR. You'll find them there: https://github.com/Unvanquished/unvanquished-mapeditor-support/tree/master/build/darkradiant

I'm not sure we need to set an “unvanquished” type because the map format we use is exactly the same as the quake3 one. Most Quake 3 derivatives including Unvanquished make special efforts to not break the formats anyway. It looks like all that is needed is a “legacy brush” writer being named “quake3” type, and the current brush writer being named quake3 alternate or something like that.

If DarkRadiant provides “doom3”, “quake4”, “quake3” and “quake3-alternate” types, a game like Unvanquished would just set « type="quake3" » and a game like Xonotic would just pick « type="quake3bp" »  in .game file without more game specific code. I'm not sure “quake3-alternate” is a good name but you probably see the point. And in map exporter window, people would be able to chose between “Doom 3 Map”, “Quake 3 Map”, “Quake 3 Map (alternate)”, “Quake 4 Map”. I see no need in having an “Unvanquished Map” option there.

I have no idea what would be the best name for the “quake 3 alternate“ format though.

> Unvanquished .game file refers to gametype "quake3", and the "quake3" map format writes the older brush syntax by default.

That would be the best solution, because that's basically what any game using quake3 format would expect when not knowing there are in fact two quake3 formats.
greebo

greebo

05.09.2021 13:19

administrator   ~0014333

The "Quake 3" format can now be selected from the list of map formats when using File > Export Map. Games with "quake3" game type now also default to use the Quake 3 tex def format.
This is now waiting for a round of testing.

Related Changesets

DarkRadiant: master 3bb66dd0

2021-08-29 06:24:57

greebo

Details Diff
0005711: Add unit tests checking the exported brush syntax for doom3, quake4 and quake3 formats. The Quake3 test is failing right now since it the exporter is still writing the newer brushDef syntax. Affected Issues
0005711
mod - test/MapExport.cpp Diff File
mod - test/MapSavingLoading.cpp Diff File
add - test/testutil/FileSelectionHelper.h Diff File
mod - tools/msvc/Tests/Tests.vcxproj Diff File
mod - tools/msvc/Tests/Tests.vcxproj.filters Diff File

DarkRadiant: master 8fa60e8d

2021-08-29 07:24:01

greebo

Details Diff
0005711: Split Quake3MapFormat class into two sub classes, one for Q3 ("legacy") and one for Q3 alternate (newer Q3 brushDef format) Affected Issues
0005711
mod - radiantcore/map/format/Quake3MapFormat.cpp Diff File
mod - radiantcore/map/format/Quake3MapFormat.h Diff File

DarkRadiant: master 795f65f8

2021-08-29 07:26:42

greebo

Details Diff
0005711: Add unit tests checking the exported brush syntax for Quake 3 alternate (using the "non-legacy" brushDef syntax) Affected Issues
0005711
mod - test/MapExport.cpp Diff File

DarkRadiant: master 9979ea11

2021-08-29 07:42:19

greebo

Details Diff
0005711: Add separate Q3 map writer. Move common double exporter code to shared header file. Affected Issues
0005711
mod - radiantcore/map/format/Quake3MapFormat.cpp Diff File
mod - radiantcore/map/format/Quake3MapWriter.h Diff File
mod - radiantcore/map/format/primitivewriters/BrushDef3Exporter.h Diff File
mod - radiantcore/map/format/primitivewriters/BrushDefExporter.h Diff File
add - radiantcore/map/format/primitivewriters/ExportUtil.h Diff File
mod - radiantcore/map/format/primitivewriters/PatchDefExporter.h Diff File
mod - tools/msvc/DarkRadiantCore.vcxproj Diff File
mod - tools/msvc/DarkRadiantCore.vcxproj.filters Diff File

DarkRadiant: master a34ace15

2021-08-29 07:45:06

greebo

Details Diff
0005711: Add unimplemented legacy Q3 brush def exporter Affected Issues
0005711
mod - radiantcore/map/format/Quake3MapWriter.h Diff File
add - radiantcore/map/format/primitivewriters/LegacyBrushDefExporter.h Diff File
mod - tools/msvc/DarkRadiantCore.vcxproj Diff File
mod - tools/msvc/DarkRadiantCore.vcxproj.filters Diff File

DarkRadiant: master 0d4c0324

2021-08-29 08:14:01

greebo

Details Diff
0005711: First Q3 brush exporter implementation Affected Issues
0005711
mod - include/ibrush.h Diff File
mod - radiantcore/brush/Face.cpp Diff File
mod - radiantcore/brush/Face.h Diff File
mod - radiantcore/map/format/primitivewriters/LegacyBrushDefExporter.h Diff File
mod - test/MapExport.cpp Diff File

DarkRadiant: master f334b172

2021-09-05 12:08:02

greebo

Details Diff
0005711: Add unit test checking the exported Q3 brush face Affected Issues
0005711
mod - test/Brush.cpp Diff File

DarkRadiant: master 9e957eaa

2021-09-05 13:15:14

greebo

Details Diff
0005711: Port some code from GtkRadiant to write out brush faces in Quake 3 format. Angled brushes won't be correctly textured, but axis-aligned ones seem to work with this algorithm. Affected Issues
0005711
add - radiantcore/map/format/Quake3Utils.h Diff File
mod - radiantcore/map/format/primitiveparsers/BrushDef.cpp Diff File
mod - radiantcore/map/format/primitivewriters/LegacyBrushDefExporter.h Diff File
mod - test/Brush.cpp Diff File
mod - tools/msvc/DarkRadiantCore.vcxproj Diff File
mod - tools/msvc/DarkRadiantCore.vcxproj.filters Diff File

DarkRadiant: master 205c31b0

2021-09-05 13:54:15

greebo

Details Diff
0005711: Unit test debugging Affected Issues
0005711
mod - test/Brush.cpp Diff File

DarkRadiant: master 26d5c1b7

2021-09-05 13:56:55

greebo

Details Diff
0005711: Fix Quake3 export test now that the tex def is exported in a different way Affected Issues
0005711
mod - test/MapExport.cpp Diff File

Issue History

Date Modified Username Field Change
13.08.2021 17:42 illwieckz New Issue
13.08.2021 18:01 greebo Relationship added related to 0005710
13.08.2021 18:04 greebo Note Added: 0014289
13.08.2021 18:04 greebo Status new => feedback
13.08.2021 18:07 greebo Note Added: 0014290
13.08.2021 18:42 illwieckz Note Added: 0014291
13.08.2021 18:42 illwieckz Status feedback => new
13.08.2021 18:59 greebo Status new => confirmed
13.08.2021 18:59 greebo Note Added: 0014292
14.08.2021 09:08 illwieckz Note Added: 0014293
14.08.2021 09:16 illwieckz Note Edited: 0014293
14.08.2021 13:38 greebo Note Added: 0014294
14.08.2021 13:38 greebo Note Added: 0014295
14.08.2021 16:16 illwieckz Note Added: 0014296
14.08.2021 19:56 greebo Note Added: 0014297
14.08.2021 20:08 greebo Note Added: 0014298
14.08.2021 20:13 greebo Note Edited: 0014298
15.08.2021 12:48 orbweaver Severity normal => feature
16.08.2021 16:03 illwieckz Note Added: 0014302
29.08.2021 05:27 greebo Assigned To => greebo
29.08.2021 05:27 greebo Status confirmed => assigned
29.08.2021 05:28 greebo Summary Can't save Legacy Q3 brush format => Change Quake3 map exporter to write "legacy" brush syntax
29.08.2021 08:16 greebo Changeset attached => DarkRadiant master 3bb66dd0
29.08.2021 08:16 greebo Changeset attached => DarkRadiant master 8fa60e8d
29.08.2021 08:16 greebo Changeset attached => DarkRadiant master 795f65f8
29.08.2021 08:16 greebo Changeset attached => DarkRadiant master 9979ea11
29.08.2021 08:16 greebo Changeset attached => DarkRadiant master a34ace15
29.08.2021 08:16 greebo Changeset attached => DarkRadiant master 0d4c0324
05.09.2021 13:18 greebo Changeset attached => DarkRadiant master f334b172
05.09.2021 13:18 greebo Changeset attached => DarkRadiant master 9e957eaa
05.09.2021 13:19 greebo Status assigned => resolved
05.09.2021 13:19 greebo Resolution open => fixed
05.09.2021 13:19 greebo Fixed in Version => 2.14.0
05.09.2021 13:19 greebo Note Added: 0014333
05.09.2021 13:54 greebo Changeset attached => DarkRadiant master 205c31b0
05.09.2021 13:57 greebo Changeset attached => DarkRadiant master 26d5c1b7