How to do entities in a scalable way

[Skaruts]

I've been creating a few kinds of entities (trigger_once/_multi, func_door, light_omni, info_player_start) and I came to realise most of them share some basic properties and methods, and I'm starting to repeat code that I think I shouldn't be repeating.

The triggers, I simply inherited from a base Trigger class. But then I realised other entities can also trigger things (buttons, levers, or even doors), since I'm following a similar mindset to HL2 and Doom3 entities. And at the same time many of them can also be triggered. So most of them share functionalities.

So I'm at a point where I'm trying to decide how to go about this. It doesn't seem as easy as creating a BaseEntity, because the entities extend different nodes: the func_door is a StaticBody3D, the light_omni is a OmniLight3D, the triggers are Area3Ds. A BaseEntity would only extend Node3D, and that doesn't work well with different kinds of root nodes (it doesn't throw any errors (they're all technically Node3Ds), but it screws up the code completion).

One solution could be to have an inner entity property, which could carry all the entity's properties that don't directly affect the node. I'm not entirely sure it would work well, but this property could freely inherit from base entities no problem. One downside is the layer of indirection that always comes with OOP-composition (e.g. obj.entity.targetname, instead of just obj.targetname).

But I'm wondering if there aren't other solutions, maybe even simpler, that I'm not seeing.

[Skaruts]

Does the bsp scene get rebuilt at runtime? It seems like it does.

I'm experimenting with another possible solution, which is for all entities to have Node3D as root node, and I do some reparenting in _ready. For example, in the case of the door entity, it creates a StaticBody3D, sets it as child, and reparents the mesh and the collision nodes to it. So it looks like this:

func_door_rotating (Node3D)
    StaticBody3D
        mesh
        collision

However, it seems I have to run that reparenting code at runtime too. If I don't, then the during runtime the door will just look like this:

func_door_rotating (Node3D)
    mesh
    collision

Is that supposed to happen?

It's not really a big deal, I'm just wondering if it's a bug.

[jitspoe]

Yeah, I think that's kind of a fundamental issue with OOP style programming in general. I'm trying a different approach for KOOK where I'm taking just sort of the "systems" side of ECS and putting all the logic in systems. The classes on the nodes mostly just hold the data.

For example, func_door maps to my door_sliding class:

@tool
extends CharacterBody3D

@export var targetname : StringName
@export var offset : Vector3
@export var speed : float
@onready var start_position := position
var target_position : Vector3
var open := false
var moving := false
var returning := false
var fraction := 0.0

func set_import_value(key : String, value : String) -> bool:
    if (key == "speed"):
        speed = value.to_float() / 32.0
        return true
    return false

func _ready():
    set_process(false)
    set_physics_process(false)

    if (Engine.is_editor_hint()):
        return # Don't do the stuff below in tool mode.

    if (targetname):
        TriggerSystem.add_triggered_entity(self, targetname)

    if (!offset):
        var min_z = INF
        var max_z = -INF

        for child in get_children():
            if (child is CollisionShape3D):
                var shape = child.shape
                if (shape is ConvexPolygonShape3D):
                    for point in shape.points:
                        min_z = min(min_z, point.z)
                        max_z = max(max_z, point.z)
        var length : float = max_z - min_z
        offset = Vector3(0, 0, length) # Why does this work?  Shouldn't it be negative because -Z is forward?
    target_position = start_position + offset
    if (!speed):
        speed = 100.0 / 32.0

func _physics_process(delta : float):
    Movement.process_platform(self, delta)

func trigger(_body, _target):
    Movement.start_platform_movement(self)
    print("Door triggered.")

So all the triggering and movement is handled in a place that can be shared (trigger system and movement system). The door just registers with it. There is a large chunk of code there, but that's just to figure out the size of the door and could probably be put in a shared location later.

[Skaruts]

That's not a bad idea. I'm going to experiment with it too.

---

Also, do you think you could add a second pass to the plugin?

I hacked that into the bsp_reader and it seems to work quite reliably for rearranging the nodes. It's working much better than what I was attempting before.

Basically, I added:

• an array of entities, where I append each entity that is successfully created in convert_entity_dict_to_scene
• a loop through that array at the end of read_bsp, calling on_import_finished on entities that implement the method

The method on_import_finished also takes in that same array of entities. That allows each entity that has the parent property set, to search for the correct parent, and reparent itself to it. This is useful for things like a trigger that activates and moves along with an elevator, or maybe even a loot purse on a guard's belt, etc. Though a guard's purse might require some extra properties.

There might be other uses for that too.

[Yagich]

in my game, i added a new import option called postprocess_hooks which is an array of paths to scripts. those scripts are expected to implement one function, postprocess(scene: Node3D) which takes the scene as the argument. it modifies them in place. you can check it out in my fork here, along with an example postprocess script that rotates the scene 90 degrees.

it seems like a system like that (maybe not exactly as it is done in my fork, i think it might be too specific to my game) could be desired for the main branch?

[Skaruts]

That might actually be quite useful for setting up navmeshes, occluders, GI, lightmaps, etc

[Yagich]

indeed! i use it to set up collision layers/masks and to connect signals in my game.

i think it's a good compromise between ultimate flexibility with endless import settings and the addon being "just" a bsp importer. the current set of settings is a good starting point for most projects i believe, and offering more custom functionality via scripting for more specific use cases would be powerful indeed.

do you think an approach like that would alleviate this issue?

[jitspoe]

I think it might be worth adding 2 functions. 1 kind of like the set_import_value() function that executes on every node added after all data on the scene is constructed, and one that can be placed on a global import script to execute for the whole scene. Something like post_import() or post_bsp_import(). That should probably cover all cases without needing an array of functions.

[Skaruts]

I haven't been successful in using OccluderInstance3D with this plugin though. With TBLoader's geometry, that node generates occluders just fine (with the same test map). For some reason it doesn't seem to like the geometry generated by qbsp.

My test map has:

• map geometry
• a bunch of func_statics (static geometry that can be made non-solid)
• a few func_doors
• a few torches (actual 3D models)

With TBLoader's geomtry, all of that gets turned into occluders, while with qbsp geometry only the torches do. Nothing else. I don't know much about this subject, so I don't have a clue why this happens.

I took the map geometry into a new scene by itself, and the OccluderInstance3D threw an error "no meshes to bake".

---

On the other hand, navmeshes seem to bake just fine. But there's a problem:

In order to prevent triggers and the player itself, among other things, from affecting the navmesh generation, they have to be temporarily removed from the map scene during navmesh baking. But the problem is the map scene has to be a child of the NavigationRegion3D node for it to bake anything, so I'm not sure how this can be done during the bsp import process.

[jitspoe]

A lot of the funcs (groups, etc) get baked down to the worldspawn entity when compiled to a bsp file. Ultimately, it'd be cool to us the vis data in BSP for visibility, though I don't know how well that would actually work in Godot. If you want the meshes to be separated, you'd have to break them up into their own entities. Might have to do a custom one -- just make something up like "brush_mesh_entity" then make scene you map that entity into have a static body at the root.

I haven't messed with navmesh generation yet. Is it possible to make it use specific collision layers? Then you could put the triggers and other things in different collision layers.

[Skaruts]

The thing is TBLoader also makes the whole map a single mesh, so I don't suppose that should be the issue. I have a func_static entity that I use to separate certain things from the world map, but I'm using the exact same map with func_statics in the same places.

I think the only difference is TBLoader doesn't remove hidden faces and all that. So the resulting meshes are of course different, but somehow the OccluderInstance3D can work with TBLoader's but not with qbsp's.

Maybe I could experiment with some qbsp arguments, to see if they make any difference.

As for navmeshes, yea it may be possible, but I'm not sure yet. It's being quite tricky to figure out. There's options in the NavigationMesh, under Geometry, that might do the trick, but I'll have to dig deeper. One of the options involves having it parse geometry from a group, but adding things to groups in tool scripts is quite tricky as well.

[Skaruts]

No luck, unfortunately. I tried all these parameters and still can't bake occluders:

-includeskip 
-subdivide 0 
-nomerge
-nosubdivide 
-chop

-nosubdivide makes the geometry a lot simpler and cleaner, though (visible in the 3D view in wireframe mode). I suppose it's equivalent to -subdivide 0, that you mentioned elsewhere.

[jitspoe]

I haven't messed with occluders before, but I thought they worked by blocking entire meshes, meaning you'd have to have multiple separate meshes for this to work.

[jitspoe]

I've added a call to "post_import()" on all the nodes that have that function after everything gets imported. Let me know if that's sufficient.

[Skaruts]

Personally I find it handy if each entity is given access to a list of all the entities on post import (including worldspawn), as a parameter of that call. Like I mentioned earlier:

That allows each entity that has a parent property set, to search for the correct parent, and reparent itself to it. This is useful for things like a trigger that activates and moves along with an elevator, or maybe even a loot purse on a guard's belt, etc.

Seems it passes in the root node, which I think can be useful too. But that alone doesn't give easy access to the other entities (an entity could conceivably get a list of all the children of the root node, but, if any entity has already been reparented, then it won't be in that list).

---

Might also be worth noting that there were actually two different post-import passes being suggested. Mine is a per-entity post-import pass, while Yagich's is a per-map post-import pass. So my suggestion is that each entity is given a chance to rearrange whatever it needs, and his suggestion is that the map node gets a chance to do any post-import operations, like navmeshes, lightmaps, etc.

Tbh, I see value in both. At least, I don't know any other ways to achieve the same thing without two post-import passes.

[Skaruts]

I haven't messed with occluders before, but I thought they worked by blocking entire meshes, meaning you'd have to have multiple separate meshes for this to work.

I completely forgot about this... Well, the thing is, TBLoader also makes the entire map a single mesh. It only separates groups and brush entities. And in my tests I tried to keep them both the same in terms of the resulting map after import. So I really can't tell why one generates occluders just fine and the other doesn't. I mean, there are differences in geometry, since qbsp removes hidden faces and all that. But I don't know if that's what prevents occluders from happening.

I was thinking of maybe exporting the geometries and having a look in Blender. But I haven't got around to it yet. Been focusing on other stuff lately.

[jitspoe]

Personally I find it handy if each entity is given access to a list of all the entities on post import (including worldspawn), as a parameter of that call. Like I mentioned earlier:

I feel like the root node and all its children is the list of all the entities? Maybe I'm misunderstanding what you're proposing.

I didn't get around to the global post_import() thing. Wasn't really sure where it'd be best to put it. Seems like it should be some sort of global tool script, but I'm not sure how to specify that.

[Yagich]

I didn't get around to the global post_import() thing. Wasn't really sure where it'd be best to put it. Seems like it should be some sort of global tool script, but I'm not sure how to specify that.

godot's advanced importer that's used for GLTF and .blend imports allows specifying a path to a post-import script that is expected to implement one function, _post_import(scene: Node) -> Node. i think that's a reasonable approach.

[Skaruts]

I feel like the root node and all its children is the list of all the entities? Maybe I'm misunderstanding what you're proposing.

It could be, but the problem is entities can potentially re-parent themselves to other entities, and no longer be direct children of the root node, and then the next entities won't have access to them.

[jitspoe]

I suppose you could do a pre-pass to generate the list if you needed to account for that. What, exactly, are you trying to do?

[jitspoe]

Ok, there's now a post_import() called on each node (if they exist) and a script that can be specified to call post_import() on the whole scene. Hopefully this takes care of this. Feel free to re-open or open a new issue if you need something more specific.

[Skaruts]

I was thinking closer about this, and I think I lead you in the wrong direction. I think a single "global" post import would do the trick for all purposes. It would allow users to do any post import in whatever way they want, without you having to worry about twisting the plugin to fit any specific requirements.

That way I could indeed get my list of entities there, by just getting the children of the root bsp node, and do whatever I want with it, and define my own way of doing each entity's post import on my own, without you having to care about it at all.

And it would also allow setting up Navmeshes, GI, etc.

And all you'd ever have to care about is doing one single function call, with the bsp root as argument, and you're done.

I didn't get around to the global post_import() thing. Wasn't really sure where it'd be best to put it. Seems like it should be some sort of global tool script, but I'm not sure how to specify that.

That's indeed the problem, though... But I think @Yagich's approach with post_process_hooks, or something along those lines, might be a good way. The user could optionally provide a script that defines a post import function.

[jitspoe]

The global post_import() is already implemented. :)

Just throw it on a script file and specify that in the import options. Needs to be a tool script.

[Skaruts]

Thank you.

Feel free to remove the per-node post import loop you added in previously, if you want, as it's kind of redundant now.

Sorry about the confusion.

[jitspoe]

I'm actually using that one. :) For little fixups, it makes things simple, so I'll probably just keep it.