Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Feature request) Improved FGD support #115

Open
c-d-a opened this issue Aug 29, 2022 · 20 comments
Open

(Feature request) Improved FGD support #115

c-d-a opened this issue Aug 29, 2022 · 20 comments

Comments

@c-d-a
Copy link

c-d-a commented Aug 29, 2022

FGD entity definitions are nicer than DEFs/ENTs because of class inheritance. NRC already supports FGD format, but only in some strange limited form.
Currently, it requires setting entityclass="halflife" in the game settings, which causes spawnflags to disappear in the UI - something I don't see any reason for. Fixing this would be a great start.

The syntax seems very strict (one wrong-case letter and the whole file fails to load), so many existing FGDs are incompatible.
It would also be nice to be able to set the displayed skin and animation frame for models/sprites. For example, TrenchBroom supports doing something like this:

@PointClass base(Monster) 
	model({ "path" : "progs/zombie.mdl",
		"frame" : {{ spawnflags & 1 -> 196,0 }} }) = monster_zombie : "Zombie"
[
	spawnflags(Flags) = 
	[1: "Crucified"
	2: "Ambush"]
]

That would be ideal, but even simply picking up "skin" and "frame" custom properties would be fine.

@Garux
Copy link
Owner

Garux commented Aug 31, 2022

FGD support is about 2003's WorldCraft format. HalfLifers prefer hammer-alike editors mostly, so this is pretty abandoned.
TB uses own unique format of FGD.

entityclass="halflife" in the game settings, which causes spawnflags to disappear

This literally enables FGD use, nothing to do with GUI, flags are parsed:
https://github.com/Garux/netradiant-custom/blob/master/radiant/eclass_fgd.cpp#L318
Looks like they are stored in wrong way, so GUI doesn't get them, would be cool to fix.

one wrong-case letter and the whole file fails to load

this is easy fix, hint me which strings are picky to the case

simply picking up "skin" and "frame" custom properties would be fine

Not that simple under the hood, this requires modification of resources hashing system. Luckily most of games do not care about frames in mapping regard. Thing to also consider is whether Assimp has frame loading for formats of interest.
As for skins, their implementation varies a lot per game/format, Assimp supports skins for some formats, but need of unique handling in each case is expected.

FGD's strong feature is inheritance, while ENT has way more rich base of defined types and their properties, also extensible w/o backwards compatibility break. Some idea is to add way to avoid duplication in ENT, may be not exactly inheritance, but way to insert predefined block. Design wise it's not clear how to do it well, in real cases it's often wished to have unique description for certain key or control over the order of keys. E.g. child adds a key, it will end up before or after the derived block, while it's wanted somewhere near the related keys,

@c-d-a
Copy link
Author

c-d-a commented Aug 31, 2022

Predefined blocks in ENTs sound great.
The order of keys in FGDs (as it is now) inserts inherited properties below the ones unique to the entity. I think that makes sense, clearly shows how an entity differs from the rest. TB just sorts everything alphabetically, FWIW.

Re: frames, the practical use-case is placing misc_models as part of the world (characters as statues, for example). In Q1 you'd just leave it as a separate entity. In Q3 you could in theory bake it into the world, but the q3map2 documentation on entity keys just straight up says "_frame" doesn't work. At the end of the day, none of this is that big a deal of course.

this is easy fix, hint me which strings are picky to the case

The classes: SolidClass, PointClass, BaseClass
Other things it usually fails on might be TB additions. One is "model" instead of "studio" when defining the entity's model (though TB-specific FGDs will probably have extra characters in the model path, so it's going to break anyway). Another is using '=' instead of ':' as a separator, like so:

property(type) = "description"
property(type) : "description"

Even just keeping the strict syntax would be fine if it would only throw a single error with the number of the problematic line. As it is, one has to sit for a full minute holding Enter to skip through all the errors, which tends to drain the motivation somewhat.

@Garux
Copy link
Owner

Garux commented Aug 31, 2022

This part is really case sensitive for some reason
https://github.com/Garux/netradiant-custom/blob/master/radiant/eclass_fgd.cpp#L534
turning ASSERT_MESSAGEs to console errors would be helpful for managing 'bloated' fgds.

@eGax
Copy link

eGax commented Oct 4, 2022

OMG, yes it is so strict with fgds. I've tried a few of mine and a few other older ones, but it would require large rewrites to make them work.

The syntax seems very strict (one wrong-case letter and the whole file fails to load), so many existing FGDs are incompatible.

Garux added a commit that referenced this issue Oct 31, 2022
… rest of messages go to the log

report problem fgd line
parse block names case insensitively
#115
@ZecChelovek
Copy link

I understand it would be difficult but seconding this idea hard.

Garux added a commit that referenced this issue Dec 15, 2022
@RazielZnot
Copy link

RazielZnot commented Aug 25, 2023

Now that Quake 2 Remaster is released we need proper FGD support ASAP, otherwise people are forced to switch to TrenchBroom and TBH I'm not a fan of that editor because of its severe lack of features. After so many years of using Radiant family editors it's just painful to use something different...

I've tried to create my own game profile with the FGD files distributed with the game but it causes tons of errors during initial parsing and in the end the editor just crashes. Very unfortunate outcome. So there is only two choices: to use TrenchBroom or to convert FGD to DEF or ENT manually and I'm feeling the pain even when thinking about it! Or maybe a tool exist for such task? I only found the tool for converting ENT and DEF to FGD and not the other way around.

@Garux
Copy link
Owner

Garux commented Aug 26, 2023

Define proper support kinda. Classic FGD is supported, various format extensions, added by other editors are not. With referenced changes running through pointed parsing errors and fixing them must be pretty doable. I can try that, if you share the FGD.

Wrt conversion, ENT would be the best option, as it's reachest of the 3 formats. I added 2ENT part here https://github.com/Garux/def2fgd, fromFGD part is missing tho.

@RazielZnot
Copy link

Define proper support kinda.

Some unification between editors is needed. It's really strange that TrenchBroom devs decided to break the standard, this deserves extreme judgement TBH.

I added 2ENT part here https://github.com/Garux/def2fgd, fromFGD part is missing tho.

Having this tool as a plugin in the editor but with the all needed features would be great, so people could use it right away.

I can try that, if you share the FGD.

You can get them here: https://github.com/id-Software/quake2-rerelease-dll

@Garux
Copy link
Owner

Garux commented Aug 27, 2023

@RazielZnot isn't it gamepack for your game https://github.com/TTimo/Q2RePack/? Has .def.

@RazielZnot
Copy link

@Garux that's for the latest GtkRadiant and it requires manual changes to make it compatible with NetRadiant Custom. I already tried using GtkRadiant 1.6.7, and even though they added new features, but they also managed to break those that worked perfectly fine, like you can't select brush face anymore for some bizarre reason, or rotation is very coarse now and it works only in the axis of the current 2D view, maybe something else but definitely things got worse. Also in their blog they say:

The entities.def in the pack is probably a little outdated, a conversion effort from the latest fgd may be needed (or directly adding fgd support to radiant).

I don't understand the reasoning behind making everything incompatible with the established standards, it always makes things worse.

@Garux
Copy link
Owner

Garux commented Aug 27, 2023

  • can just use .def from that gamepack
  • you likely mix up GtkR 1.5 & 1.6; 1.6 is 1.4 in general
  • they had reasons to break compatibility, adding features to make things better. Though imo it should not be passed off as FGD, as it confuses apps with FGD support and their users. Having different extension would be nuff.

@RazielZnot
Copy link

can just use .def from that gamepack

I did some research, it really is very outdated but also pretty barebone, paths for the models are missing for most of the new entities. But there is also unfortunate surprise about FGD, it's incomplete as well, for example rotating_light is missing but present in the DEF. So both FGD and DEF are outdated in their own way, as if different people worked with each of them. And I checked the game's dll source code of the remaster, it contains QUAKED comments which can be used to assemble DEF file from scratch, unless the devs did a nasty job and missed some of such comments...

@Garux
Copy link
Owner

Garux commented Aug 28, 2023

Fixed errors, some warnings in the log left.
Note, some functionality was removed to get it running. Multiple spawnflags per entity (for ericw tools, not supported in radiant), flags over 16 bit (probably okay to increase the max in radiant, no games were using them so far).
quake2-rerelease-dll-main-fgd-nrc.zip

@pinksloyd
Copy link

Q2RePack is not complete. It's the same as I've had in my GTKRadiant-folder forever (or for over 20 years or so).

However, how about a real NetRadiant, one that can be accessed through the browser? ;w; I know jdolan did a JSRadiant back in the days, but not much came of it. I'd really like to have the capacity to just make maps through a browser.

@Garux
Copy link
Owner

Garux commented Aug 28, 2023

WebRadiant

Not gonna happen, unless C++ + Qt is easily portable to web tech. Or unless new hero will enter the chat.

@pinksloyd
Copy link

But but: https://github.com/jdolan/radiantjs

@SirYodaJedi
Copy link

Looking at the code, I see that the color1 KV type is missing:
https://github.com/Garux/netradiant-custom/blob/master/radiant/eclass_fgd.cpp#L461
https://developer.valvesoftware.com/wiki/FGD#Keyvalues
It has color255 (1D array of 0-255 per color, plus anything or nothing afterward), but color1 (1D array of 0-1, plus anything or nothing afterward) more accurately represents how IdTech engines tend to handle color KVs.

@Garux
Copy link
Owner

Garux commented Sep 19, 2023

It's not hard to add. Would be nice to know which dialect this color1 comes from though. Q2re fgds use unknown color key type for both byte & float colours.

@SirYodaJedi
Copy link

SirYodaJedi commented Sep 19, 2023

Would be nice to know which dialect this color1 comes from though.

Seems to always have been a thing; it can be found in old Worldcraft 1.6 FGDs.
https://developer.valvesoftware.com/wiki/Quake_II.fgd

Q2re fgds use unknown color key type for both byte & float colours.

Seems to be a TrenchBroom thing; TrenchBroom doesn't properly detect either color255 or color1 yet.

Garux added a commit that referenced this issue Sep 20, 2023
@Garux
Copy link
Owner

Garux commented Sep 20, 2023

Adjusted adjustments to the latest commits.
quake2-rerelease-dll-main-fgd-nrc2.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants