NSSTC / sim-ecs

Batteries included TypeScript ECS
https://nsstc.github.io/sim-ecs/
Mozilla Public License 2.0
88 stars 13 forks source link

Improve running systems declaration #33

Closed iamcsharper closed 3 years ago

iamcsharper commented 3 years ago

Currently, each state defines which systems will be running As soon as you reach 50+ systems it gets really annoying to keep your eye on both GameState and createWorld() function

I really wish there were a more versatile way of assigning systems to state, not the vice versa for example, using decorators

@RunsInStates([ GameState, MenuState ]) export class MenuSystem extends System { readonly query = new Query({ uiItem: Write(UIItem) });

or simply a readonly array

export class MenuSystem extends System { readonly _states = [ GameState, MenuState ]

I know it can also become messy if one wants to program your world in a state-first way, not the system-first one But then there should be an "all systems" mechanism in states, which would work by reading all the unique systems from ECS world

minecrawler commented 3 years ago

As soon as you reach 50+ systems it gets really annoying to keep your eye on both GameState and createWorld() function

Yes, and it's a problem which directly conflicts with the use-case of sim-ecs. There is a work-item in the "New Features" project, which waits for the bevy_ecs centralized_run_criteria RFC. I am aware of the shortcomings of the current situation, however I want to derive some of the ideas from bevy to implement a solid solution. Once they move forward, I want to take a look at how to make run criteria (as in which systems should run when, including the state) easy to set up.

using decorators

Decorators are a boon, however they are still in draft and experimental, which is why I refrained from adding them. Once they are finalized and not behind a feature-flag anymore, I will probably add some to remove boilerplate where possible.

or simply a readonly array

Since there are fewer states than systems, this could be a short-term fix. I will look into it for 0.4!

vmwxiong commented 3 years ago

For what it's worth, decorators have been around for a fair bit now, and I have seen them used effectively already. I know it's not ideal, but I'm guessing that if changes are made they will be minor, and given the state of this project, it might be worth integrating now and just dealing with the change if and when it comes. Might help clarify some design decisions along the way, just my two cents.

minecrawler commented 3 years ago

@iamcsharper I changed the way systems are assigned to states. It now works as per your request. The changes are only in the master branch, but will be included in the next release 😉

// State definition
class GameState extends State {
    activate() {
        console.log('Game State is active, now!');
    }

    deactivate() {
        console.log('Game State is inactive, now!');
    }
}

// System definition
class GameSystem extends System {
    readonly query = new Query({ /* ... */ });
    readonly states = [GameState];

    run() { /* ... */ }
}

// Dispatch a state once
world.dispatch(GameState);

// OR start running in a state
world.run({ initialState: GameState });

By the way, if you want to run all systems, you can do so by omitting the initial state, or if you want to switch from a state to running all systems, you can simply pass State as state. If you created the world using buildWorld(), the state State was automatically populated with all registered systems.