Garux / netradiant-custom

The open-source, cross-platform level editor for id Tech based games.
https://garux.github.io/NRC/
Other
334 stars 59 forks source link

Quake 2 Valve220 breaks when editing and saving .map #84

Open motorsep opened 3 years ago

motorsep commented 3 years ago

I have Quake 2 Valve220 map and every time I add something to it or modify anything on the map and then save it, it breaks the map file badly.

Does NetRadiant Custom even support loading/saving Quake 2 Valve220 maps ? Thanks

Garux commented 3 years ago

Yeah, it's supposed to work. Works here with a test map: image What exactly gets broken for you? Any test map to reproduce? We've recently caught crash during writing a map with nearly infinite texture projection, which was borking a .map also. Here is wip build with it fixed: https://www.dropbox.com/s/uqq46o8grlrolu4/netradiant-custom-wip.7z?dl=1

Only problem i've noticed so far: surf/cont flags are not displayed correctly by surface inspector for the loaded v220 map, while modified in runtime are. Same is true for BP map.

Garux commented 3 years ago

Ok, found fix for the latter, but there is one more obscure problem. Flags are always written to .map with BP/220, even default ones. How does compiler handle this, when there is also texture with embedded flags applied?

Also why are we looking into this, are there Q2 mappers, using nrc? There might be need to have unset flags function to support compiler's way of handling to make flags thing usable properly.

motorsep commented 3 years ago

@Garux I am working on a project using Q2RTX - converting some of the best maps for Quake 1 into Quake 2 to be rebuilt for Q2RTX using qbism's q2tools (which recently got extended q2bsp format - QBSP, akin BSP2 for Quake 1).

Here is an example of one of the converted maps: https://www.youtube.com/watch?v=UBmeh5YrV68

I have that one map, spmaxx, which is Valve220 map. It got converted to Q2 .map format nicely and loads up into NetRadiant Custom just fine. However, of I change anything on it (like add a brush or adjust a brush or turn brush into detail/structural, etc.) and save it, it breaks. It breaks in a sense where some func entities "explode" with most of the brushes comprising it get moved outside of editor bounds. 4bsp compilers quits because of some "brush 1 - floating plane inverted normals" issue.

Note that if I compile that converted map right away, without loading it into NetRadiant, it compiles just fine (but leaks).

I am at work right now, but I can upload that map for you to try and see for yourself.

As for surfaces flags, it looks like I had mostly no issues with other maps (standard, non-Valve220 converted maps). It seemed that NRC loaded surface and content flags from .wal files perfectly fine for the most part (there were some rogue cases where some clip brushes had weird totally random surface/contents flags set). At least NRC doesn't corrupt the .map as Trenchbroom does when loading Quake 2 maps :P

P.S. as for other Q2 mappers, there is Nail & Crescent teams that might use NRC. Also with this new QBSP format and Q2RTX support added to it last night, perhaps we'll see more Q2 mappers coming around.

Garux commented 3 years ago

Right, it's would be logical to check with problematic map, as idk how such scary things can happen in general.

motorsep commented 3 years ago

@Garux There ya go: https://drive.google.com/file/d/1gunh6uFjKaDhXHV_7j9NJTFT3dzelspu/view?usp=sharing

Here is a video showing the issue: https://www.youtube.com/watch?v=76EwGk7Ep48

Garux commented 3 years ago

Well, very interesting. Nothing like that happens here after reproducing steps exactly. Resaved map file i'v got: sp_maxx.zip

Garux commented 3 years ago

Ah ok, was blind guess, but i ticked this option image and reproduced the problem. Don't use it w/o knowing what it does really!

motorsep commented 3 years ago

I have no idea what that is - I didn't even bother with options. Probably it's checked by default ?

Yep, that did the trick. Thanks!

Garux commented 3 years ago

No, it's off by default and has no options to default it.

Garux commented 3 years ago

Should probably remove this snap option altogether, as it's only required for very ancient compilers, not supporting float input, and does not do any deep thinking, but just snaps to int.

As for flags issue, radiant was writing them unconditionally in BP, 220 formats, just like TB. Fixed: https://github.com/Garux/netradiant-custom/commit/b58408344c2684245594ecd461f0bcb4eee99ea3 Also added unset flags function: https://github.com/Garux/netradiant-custom/commit/8d71aea0595e08fc036c3578581d8704cffc1f4c build: https://www.dropbox.com/s/uqq46o8grlrolu4/netradiant-custom-wip.7z?dl=1

Flags editor is not very precise still, e.g.: -no newly set detail flag display, if flags are not specified (ok after map reloading, flags toggle) -making brush with .wal flags detail still displays the former, while only detail flag is there in fact (ok after map reloading, flags toggle) Detail logic is separate from flags, probably because it's wanted to be only applied to whole brush, this requires deep analysis to be resolved gracefully. Tell me, if there are more of problematic cases to consider.

The-Gig commented 3 years ago

Maybe, instead of completely removing the option, adding a note like "Dangerous. Only enable if you know what you are doing.", with a mouseover tip like "Required only for very old compilers. MIGHT BREAK YOUR MAP FILE." (text just for example, you can surely find better wording)?

Garux commented 3 years ago

There is no much space for text, by design it's just one line. Cool addition would be tooltips with extended descriptions, but nobody reads tooltips :) I think the way is to remove and see, if anybody still needs it, it's easy to revert removal. Option is quite crappy anyway, as planepoints are not necessarily in touch with winding points; snapping these may result in significant precision loss.

The-Gig commented 3 years ago

Then, just a "DANGEROUS"? (Otherwise, remove it..)

motorsep commented 3 years ago

@Garux Is there any chance to get new release binaries for Windows ? 😊

Garux commented 3 years ago

@motorsep i've linked fresh build above in the text

motorsep commented 3 years ago

@Garux Unfortunately nothing changed:

quake039

All I did was I changed texture on one side of a brush to "skip" and set flag to NODRAW. Saved the map, recompiled it and when I loaded the map, not only I see all the brushes that I shouldn't see, but also player spawned outside of the map :(

Garux commented 3 years ago

Need more concrete details on this, like 'i take this clip brush without flags, save, get brush with unwanted flags written', because i've just tested with the linked build + your .map, and it operates with flags as expected.

motorsep commented 3 years ago

ugh, it might be damn qbism tools ruining it :( I'll do more tests with different build first.

motorsep commented 3 years ago

Yep, damn tools are messing up the map/bsp. Sorry @Garux , I can't even test this build of NRC without being able to compile the map :(

motorsep commented 3 years ago

@Garux alright, it was indeed an issue with 4bsp and now that it's fixed, I can confirm that so far that build of NRC (above) works fine. I opened the map, did some changes (assigned texture, flagged it as nodraw/mist), saved the map, compiled it and it looks as expected. Haven't done extensive testing though.

motorsep commented 3 years ago

There ya go, the result (I was fixing vine brushes):

https://www.youtube.com/watch?v=VmaaTgbatiY

RazielZnot commented 3 years ago

Should probably remove this snap option altogether, as it's only required for very ancient compilers, not supporting float input, and does not do any deep thinking, but just snaps to int.

Don't do this. I use it very often exactly for the sake of backward compatibility with old compilers and editors.

Garux commented 3 years ago

Aww, so do they fail to read .map file with planepoints in float? Commonly used atoi function seems to work: https://godbolt.org/z/KfqceK3q7