First pass at entity definition system.

[distractedmosfet]

I whipped this up for my own uses but might as well offer it back to the project. There's still enhancements that can be done but it is usable. There's probably some last things I'll tweak/investigate but I thought I'd open the PR now to get feedback sooner rather than later.

This PR adds two new qodot nodes: QodotEntityDefinition and QodotEntityDefinitionSet. These two are used to build a scene that describes a set of entity defs you want to export to a FGD for use in TrenchBroom. The idea is straight forward enough: make a new scene, make the parent node a QodotEntityDefinitionSet. Add QodotEntityDefinition children. Set the exported variables on each definition appropriately, the most important detail is to set the path to a corresponding subscene file that you want to use as an entity. After adding your children, set the export file path on the QEDS to the FGD you want to write over, and use the pseudo-button to export it.

The exported entities are currently pretty simple. Notably there is no exported mesh or icon.

To use an entity definition set in a QodotMap, there is a new exported variable on Qodotmap to select a QEDS file. When you reload the map the entities that are recognized within your selected QEDS will be replaced with instanced child scenes.

A particular thing I'd like feedback on is my use of the InstancedScene class in build_entity_spawns_step.gd. Fundamentally the problem I was encountering there was trying to communicate to qodot_map that this node was an instanced subscene. This is relevant because qodot_map resets the ownership of added nodes recursively. This causes all the subscenes children to be injected in the scene too, which is confusing compared to standard Godot behaviour. I am not sure if there is a more ideal way to communicate that information back up the call tree, so I thought I'd just ask.

Another small detail is that I've opted simply to use UNIX-style '\n' newlines in the exporting FGD. But that might be in-ideal for Windows users. GDScript doesn't seem to offer a straight forward way of getting the current platform's newline value. I'm open to suggestions on what/if to do anything about it.

[Shfty]

Thanks for this- it's always nice to see contributions! And apologies for the slow response to your post on the issue tracker, it's been a busy week.

This PR adds two new qodot nodes: QodotEntityDefinition and QodotEntityDefinitionSet. These two are used to build a scene that describes a set of entity defs you want to export to a FGD for use in TrenchBroom. The idea is straight forward enough: make a new scene, make the parent node a QodotEntityDefinitionSet. Add QodotEntityDefinition children. Set the exported variables on each definition appropriately, the most important detail is to set the path to a corresponding subscene file that you want to use as an entity. After adding your children, set the export file path on the QEDS to the FGD you want to write over, and use the pseudo-button to export it.

Interesting- my initial plan for entity definition export was to use Resource-derived types to represent the map editor plugin (to cover editor-specific stuff like TB's game icon and cfg file), FGD file, and individual entities defined inside the FGD, then have the user configure them through the inspector as type-hinted array exports.

It sounds like your method implements this as far as the FGD iself being represented as a QEDS with baked-in export functionality, and having read through your initial proposal you do make a good argument for using a scene tree-driven system to define the entities themselves. On the subject-

A particular thing I'd like feedback on is my use of the InstancedScene class in build_entity_spawns_step.gd. Fundamentally the problem I was encountering there was trying to communicate to qodot_map that this node was an instanced subscene. This is relevant because qodot_map resets the ownership of added nodes recursively. This causes all the subscenes children to be injected in the scene too, which is confusing compared to standard Godot behaviour. I am not sure if there is a more ideal way to communicate that information back up the call tree, so I thought I'd just ask.

I need to familiarize with the specifics of InstancedScene itself, but I think supporting instanced scenes to some degree is inevitable for a proper point entity workflow, and it sounds as if your TileSet-like implementation plays into that. I've not had a chance to pull the code down and put the changes through their paces yet, so I'll give that a shot over the weekend and let you know what I think.

The QodotMap ownership change code to move things into the editor tree is pretty perfunctory at the moment, so extending it with a proper heuristic for adding editor-addressable children is probably the best bet- it's something I've been loosely planning for a little while anyway, since scene tree complexity seems to have major in-editor performance implications, and the atlased mesh implementation has to hide its children from the editor to work around a Godot-side serialization bug.

Also, I'm not sure if this pull covers brush entities like func_door, but it's worth noting that they're a unique case since their geometry is defined in the map file and augmented with parametrized script-like logic. My intent is to look at doing a simple classname > script mapping for those, since building an instanced scene and slotting in a mesh would be overcomplicating it compared to the Q1 workflow.

Another small detail is that I've opted simply to use UNIX-style '\n' newlines in the exporting FGD. But that might be in-ideal for Windows users. GDScript doesn't seem to offer a straight forward way of getting the current platform's newline value. I'm open to suggestions on what/if to do anything about it.

I'll test that and see if it has any implications on Windows. Worst-case, adding a global function to do an OS check against the Godot API (presuming it has such a feature) and using a match statement to output a platform-appropriate newline would probably suffice.

[distractedmosfet]

Thanks for this- it's always nice to see contributions! And apologies for the slow response to your post on the issue tracker, it's been a busy week.

No worries! I assumed something like that. I had my own reasons to make myself a solution sooner rather than later, and thought it might just be easier to weigh up the idea if I had the code to show.

Interesting- my initial plan for entity definition export was to use Resource-derived types to represent the map editor plugin (to cover editor-specific stuff like TB's game icon and cfg file), FGD file, and individual entities defined inside the FGD, then have the user configure them through the inspector as type-hinted array exports.

It sounds like your method implements this as far as the FGD iself being represented as a QEDS with baked-in export functionality, and having read through your initial proposal you do make a good argument for using a scene tree-driven system to define the entities themselves. On the subject-

Yeah this system only addresses FGD creation. And I wouldn't be surprised if it needed to be made to/replaced by something to fit into a more full system for specifying a complete TrenchBroom project.

The thought of this vs Resource-based stuff also ran through my mind. I decided to reach for the solution that was very obvious to me, and didn't feel too alien (as I had mentioned, it lines up with the Convert To->Tile Set feature that Godot already offers). However obvious doesn't mean best, and it is worth still weighing up against a Resource approach.

I have yet to play with custom Resource building myself, so I'm not sure of the range of flexibility there. I am aware of some limitations and on-going work on the subject, but not enough to conclusively say if I think its a better or worse approach.

It is worth noting that obviously you don't have to be hasty. My code is here to look at and weigh up, potentially to be used by the impatient, but there is a respectable logic in waiting while you consider whether you want to accept this approach. I haven't considered other approaches too deeply. It was simply quickly at hand.

Also, I'm not sure if this pull covers brush entities like func_door, but it's worth noting that they're a unique case since their geometry is defined in the map file and augmented with parametrized script-like logic. My intent is to look at doing a simple classname > script mapping for those, since building an instanced scene and slotting in a mesh would be overcomplicating it compared to the Q1 workflow.

I should have stated: this does not address brush entities at the moment. If I was adding it to this approach I would probably use a separate node, creating I guess a QodotPointEntityDefintion and a QodotBrushEntityDefintion. Since one would ideally enforce picking a tscn/scn and the other a script.

I'll test that and see if it has any implications on Windows. Worst-case, adding a global function to do an OS check against the Godot API (presuming it has such a feature) and using a match statement to output a platform-appropriate newline would probably suffice.

I had looked for such function for getting the OS and failed but now have managed to find it: it's OS.get_name() I'll probably make that edit sometime in the coming days.

[distractedmosfet]

Also one thing: while there's definitely a nicer sense of order in the idea of using a Resource, I suppose one thing that the Scene approach does have going for it is that I imagine if you had a large set of entities to consider, being able to group categories under intermediate nodes and use the search bar to find a definition by name would presumably make life easier.

Part of me is tempted to go the full TileSet route. TileSets are Resources in Godot and there is a Resource editor for them there's just also this separate optional workflow for creating them as a Scene and converting it to a Resource, and Qodot could do the same. But I don't know if that's supporting two things to enable greater flexibility, or indecisiveness.

[Shfty]

Yeah this system only addresses FGD creation. And I wouldn't be surprised if it needed to be made to/replaced by something to fit into a more full system for specifying a complete TrenchBroom project.

The thought of this vs Resource-based stuff also ran through my mind. I decided to reach for the solution that was very obvious to me, and didn't feel too alien (as I had mentioned, it lines up with the Convert To->Tile Set feature that Godot already offers). However obvious doesn't mean best, and it is worth still weighing up against a Resource approach.

I have yet to play with custom Resource building myself, so I'm not sure of the range of flexibility there. I am aware of some limitations and on-going work on the subject, but not enough to conclusively say if I think its a better or worse approach.

Resource does have its issues, but they mostly focus around export(Resource) having a bad inspector workflow with huge dropdowns. There are ways to work around that, such as having the user drag-drop resource files directly from the FileSystem tab, or grabbing all the resources of a given type from a specified search path and populating an array with them. Once the data is actually in place, editing via the inspector works as you'd expect it to:

(I did some experimenting by setting the base classes for QodotEntityDefinition and QodotEntityDefinitionSet to Resource, and adding an export(Array, Resource) to hold child entities)

So Resource isn't perfect, but I think the structural and organizational benefits of having entity files that you can save as res/tres and manipulate directly from the FileSystem tab outweigh the downsides, and using Godot's first-class type also future-proofs data-driven systems for upgrades coming in 3.2 and 4.0.

As far as defining an FGD via tree structure goes, I was working under the assumption that the instances themselves would be stored in the entity definition scene for the sake of visualization, which is one of the key benefits TileMap and GridMap get from invoking the scene tree approach. Since the entity scenes are referenced by scene file path, they're effectively a plain-data entry in an 'entity data' struct, which doesn't require or take advantage of the structure provided by the scene tree.

That's the thing about using a scene tree for data storage- nested structures with variable child counts are a great use-case, but FGD is effectively a flat array of entity class entries contaning plain data properties. You could in theory leverage it by treating the tree as a domain-specific language describing the FGD class hierarchy and inheritance spec, but that's pretty complex in comparison to a simple base classes string array in a Resource type.

Also one thing: while there's definitely a nicer sense of order in the idea of using a Resource, I suppose one thing that the Scene approach does have going for it is that I imagine if you had a large set of entities to consider, being able to group categories under intermediate nodes and use the search bar to find a definition by name would presumably make life easier.

The resource workflow covers this use case as well- the user can save each entity def to its own file and organize / search them arbitrarily using the FileSystem, then aggregate them into another 'entity set' file and plug that into a further 'map editor plugin' file for export.

Part of me is tempted to go the full TileSet route. TileSets are Resources in Godot and there is a Resource editor for them there's just also this separate optional workflow for creating them as a Scene and converting it to a Resource, and Qodot could do the same. But I don't know if that's supporting two things to enable greater flexibility, or indecisiveness.

Going full TileSet and implementing equivalent custom editor UIs to manage custom datatypes is pretty ambitious. While I do think that custom UI stuff is the ideal for any plugin with its own data flow, I don't know if raw GDScript is up to the task- as far as I'm aware, doing that involves creating an editor module and compiling the entire engine locally, which is a bit beyond the scope of Qodot right now.

So overall I think involving a scene tree and spatial hierarchy is a bit too manual, and goes against Godot's established semantics of Resource types managing FileSytem assets and game-specific data. It can't be merged as-is, but the data layout and base export workflow you've implemented work well and are a solid starting point for a more involved implementation covering the extra features discussed above.

If you're up for changing a few things to make it fit into a Resource-style paradigm that can have the other stuff built around it, I'm happy to do a code review on the relevant areas and start putting together some reference info in #8.

[distractedmosfet]

So I tried out changing it over to Resources and giving it a play around.

There are elements that I felt nicer about it as a Scene file, but almost all of that was the current inability to export custom Resource types. I think it's likely safe to assume that that limitation will be addressed in Godot, and it'd probably be better to release a workflow that is more future-proof, even if slightly more annoying right now.

I've updated my fork to now use Resources. Additionally I took the time to implement the newline thing as previously discussed, and I renamed QodotEntityDefinition to QodotPointEntityDefinition, with the idea being that I imagine in the future it'd be best to split out to QPED and a QodotBrushEntityDefinition, and doing the rename now means not killing all existing entity definitions later down the line when they all stop pointing to the right script.

Hopefully that's the bulk of what's needed to make this work useful to the project. Feel free to get back to me with more suggestions/concerns.

[Shfty]

Hey, apologies for the slow response- I've reviewed the changes and they're looking good, though there are a few small tweaks to organization and variable naming that I've noted above.

Once those are sorted, this should be ready to merge.

[rand0m-cloud]

Great work @distractedmosfet! I added defining entities property to be exported and applied on map import. I would like to help bring this feature to completion. https://github.com/winadam/qodot-plugin/commit/089f13e3fe5a664c740ef29fc0bc72958c2cf1b0

Edit: I didn't add all the properties types. I am also not sure how to implement choices, if anyone wants to suggest a good way.

[Shfty]

It looks like there's a merge conflict on account of merging #68, can you fix that up also @distractedmosfet?

@winadam I appreciate the contribution, though these changes might be better suited to their own pull request when this one is merged since they'll need to be reviewed separately and branching off of in-progress code is likely to complicate things. Can you elaborate on what your changes entail?

[distractedmosfet]

@Shfty Maybe I'm misunderstanding you but I can't see where these desired tweaks you mentioned are?

I have updated the code so that the PR now merges nicely again.

[Shfty]

@distractedmosfet Whoops, I dialed in the review but then forgot to submit. You should be able to see it now.

[distractedmosfet]

@Shfty These things happen. I've made the desired changes.

[Shfty]

Grand, merging now. Thanks very much!

[distractedmosfet]

Thank you for your time and your explanations! It was a pleasure contributing.