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

Allow add/remove systems dynamically #124

Closed
metaphore opened this issue Nov 26, 2023 · 5 comments
Closed

Allow add/remove systems dynamically #124

metaphore opened this issue Nov 26, 2023 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@metaphore
Copy link
Contributor

metaphore commented Nov 26, 2023

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
Copy link
Owner

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:

  • When you add/remove systems during a world update call we need to be careful with the systems dataset because usually modifying a dataset while iterating is not allowed and crashes, or does things incorrectly like:
    • Should a system that gets added during iteration still be called? Or skip it until the next update call?
    • Same question with systems that get removed
  • I personally don't like init functions in libraries. The constructor should be the place to initialize the object correctly and is in my opinion easier to understand and debug than a separate init function that gets called by the library/framework. I know that sometimes there is no way around that (like e.g. components in React/Angular) but in Fleks I don't see the need for that and I prefer the constructor.
    • Since systems get called at the end of the world configuration, you will have all objects available via dependency injection. Therefore, I don't see your point of "required objects may not be available". If they get created lazily somehow like special game objects then I'd suggest to implement an event system in your game and react to those specific events.
    • Also, a system should never directly access another system (your second point). From what I know this is a very bad practice and should not be done in ECS. Systems should be modular and independant.
  • If an IteratingSystem gets created in the middle of your game then this can have a significant performance spike. It is the same reason as creating a family in the middle of your game because when that happens, we need to iterate through ALL entities and check if they are part of the new family or not. When you have thousands of entities this will most likely end up in a low fps spike.

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
Copy link
Contributor Author

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 🙂

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
Copy link
Owner

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:

  • the RenderSystem that needs the camera to know how to render things (=pass the matrix to tiledmap renderers / spritebatch)
  • a CamerySystem that keeps the camera focused on a single entity that has a CAMERA_FOCUSED tag/component

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
Copy link
Contributor Author

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 Quillraven added this to the 2.6 milestone Nov 27, 2023
@Quillraven Quillraven added the enhancement New feature or request label Nov 27, 2023
@Quillraven
Copy link
Owner

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

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

No branches or pull requests

2 participants