Quillraven / Fleks

Fast, lightweight, multi-platform entity component system in Kotlin
MIT License
174 stars 19 forks source link

Allow add/remove systems dynamically #124

Closed metaphore closed 9 months ago

metaphore commented 9 months ago

While not a strictly critical issue, the absence of system set re-configuration is quite unpleasant. The option to disable systems is good, but sometimes it's preferable to spawn and kill systems entirely. In Ashley, it usually goes hand in hand with EntitySystem#addedToEngine(Engine)/EntitySystem#removedFromEngine(Engine) and is very convenient for resource management. Systems do not always operate over entities (e.g. IntervalSystem).

One good use case could be to toggle debug rendering for a limited time. Here's what it could look like:

class DebugRenderSystem : IteratingSystem(family { ... }) {
    val heavyRenderBatcher = new()
    val heavyInternalResource = new()

    // ...

    override fun onDispose() {
        super.onDispose()
        heavyRenderBatcher.dispose()
        heavyInternalResource.dispose()
    }
}

Systems like this borrow unnecessary resources when they are not enabled and it's easier to just spawn and destroy them when needed. Conditionally creating and accessing such systems during world initialization may look rather dirty too.

This also highlights a little problem of missing a mirrored pair to onDispose() (onInitialize()?). It's always good to have such mirrored events to acquire/free resources. The system's constructor is not always the best place, as the world is not yet initialized at this point and some required objects may not be available yet (like other systems, that get added after the current one). In general case, it would be good to have that onInitialize() method right before the first onUpdate() hits. But that of course can be achieved without the library modification.

And lastly, dynamically modifying systems also brings the necessity of order control. Currently, the system set is static, the order is pre-defined, and there's obviously no need for extra sorting. But if systems can be added later, obviously, having the option to put them in the right place is crucial.

Quillraven commented 9 months ago

The problem with adding/removing systems dynamically is that it will perform slower than the current solution and you also get some additional tricky situations that can lead to hard to understand bugs. Just to mention a few points while I am against such a change:

In general, in my opinion, this change makes it way messier and users can create buggy scenarios that will be hard to understand and solve. I think it is easier to initialize everything at the start and then you know what you are working with and how things are executed.

I do agree that a DebugSystem is a special case here but to be honest ... it is a DebugSystem and anyway not crucial for your productive game in the end. Just comment out the line in the systems {} block when you don't need it and are afraid of the resources that it consumes. In an ideal setup this would anyway be covered by a test scenario in your test sourceset and there you just add the DebugSystem and don't need to care about resources since it is a test.

I hope you don't get my comment the wrong way. I do appreciate such questions/proposals and I hope I could clarify why Fleks currently is working the way it does. Let me know if I could convince you or if you still think we need a more dynamic approach of adding/removing systems. In that case please provide another example besides a DebugSystem where this is needed. Maybe there is an alternative/better approach.

metaphore commented 9 months ago

No worries Simon, I appreciate the details and a good discussion! To me, it's clear that the library is built around specific design principles, and here I'm trying to understand if it's possible somehow to achieve a specific scenario while not breaking things or over-complicating the current implementation. The performance of the lib is a big deal, and it's best to keep it efficient over being too fleksible :slightly_smiling_face:

So speaking of add/remove systems "will perform slower" and "get some additional tricky situations". I think keeping it simple as just not letting to modify systems (throw an exception) during the update loop will give a good balance between the speed and flexibility of the world setup. That way it should not introduce too much of possible edge-case side effects to handle while keeping the system iteration performance similar to the current state.

This still requires order control (priorities?), but at least sorting is done only once after the modification or initial setup so that should not slow things down linearly. And having that performance spike of a new IteratingSystem is a reasonable tradeoff, but at least you have a choice and may pick the best option for yourself.

And to give a bit more of an explanation about the cross-system referencing. It's clear, that in ECS philosophy everything is an entity. But for the sake of simplification, I find it very convenient to mix in some non-conventional techniques to better adjust to the specific game needs. Like, imagine in a simple game you have the only world camera. A usual way would be to create a unique component/entity but why introduce so much clutter if the camera is pure singleton? The obvious way is to keep it somewhere outside the ECS and update it separately. But then how do you synchronize camera updates with dependent ECS systems? In that scenario, the only option is to update the camera before or after the ECS world update. At this step, it's logical to wrap the camera update within an IntervalSystem. But then why keep the camera outside of the system, if the system on itself is a singleton (within ECS) and just use it for storage/access too (especially since Fleks has a fast system accessor). So you see where I'm going? I just like to keep everything within systems, even if they have nothing to do with entities, they still are an important part of the whole engine and take their specific order to update to be properly orchestrated with the rest of the engine.

It's a good point that systems should be modular and non-interconnected, but if those are essential ones, they should be present in the world anyway, and keeping their own resources with public access is somewhat cleaner sometimes than having them stored somewhere in global space and provided through conventional mechanisms (injection, components, etc).

Quillraven commented 9 months ago

I never implemented a Camera as an entity but sounds like an interesting approach. For me, it was always like you described it. I have a camera for my game and it is an injectable of the world. I usually then have at least two systems that are working with the camera:

Anyway, I still didn't understand where the flexibility of adding/removing systems is over enabling/disabling them. My games are usually organized in screens that are responsible for a specific part of the game. E.g. a CombatScreen which is responsible for characters fighting against monster, a InventoryScreen to manage your inventory and a DungeonScreen to handle dungeon crawling of a character party (just some random made up example). For those screens (=logical parts of the game), I need certain logic (=systems) that are known from the start. They are ALL needed but maybe not active all at once.

Is the main reason to free heavy resources for a system that is currently not needed? Or do you really have a use-case where you need to swap the order of systems around during runtime?

metaphore commented 9 months ago

Alright, I think you convinced me that in general case there's always a workaround available with current features Fleks offers.

The only case I can think of at the moment would be to add/remove debug systems. But I can clearly do it through the system's enabled flag. The only thing that would be nice to have is some kind of onEnabled()/onDisabled() state change virtual methods to react to those events from within the system itself.

Quillraven commented 9 months ago

the virtual methods make sense to me and sounds like a good addition. I should have time to bring them to the SNAPSHOT within this week