View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0005711 | DarkRadiant | Saving and loading | public | 13.08.2021 17:42 | 02.04.2022 05:55 |
Reporter | illwieckz | Assigned To | greebo | ||
Priority | normal | Severity | feature | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 2.13.0 | ||||
Fixed in Version | 2.14.0 | ||||
Summary | 0005711: Change Quake3 map exporter to write "legacy" brush syntax | ||||
Description | When 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 Information | If 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 ) ``` | ||||
Tags | No tags attached. | ||||
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? |
|
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. |
|
> 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). |
|
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. | |
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. |
|
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. |
|
At any rate, thanks for doing the leg work and checking out the game packs. | |
> 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. |
|
> 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). |
|
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. |
|
> 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. |
|
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. |
|
DarkRadiant: master 3bb66dd0 29.08.2021 06:24 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 29.08.2021 07:24 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 29.08.2021 07:26 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 29.08.2021 07:42 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 29.08.2021 07:45 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 29.08.2021 08:14 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 05.09.2021 12:08 Details Diff |
0005711: Add unit test checking the exported Q3 brush face |
Affected Issues 0005711 |
|
mod - test/Brush.cpp | Diff File | ||
DarkRadiant: master 9e957eaa 05.09.2021 13:15 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 05.09.2021 13:54 Details Diff |
0005711: Unit test debugging |
Affected Issues 0005711 |
|
mod - test/Brush.cpp | Diff File | ||
DarkRadiant: master 26d5c1b7 05.09.2021 13:56 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 |
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 |
02.04.2022 05:55 | greebo | Status | resolved => closed |