API Design : Why so much OOP ?

[Felecarpp]

The today esper API is class based programming. I think they are a unreasonable use of classes in the case of a python lightweight library. This API looks like this (from README.md).

import esper

class Position:
    def __init__(self, x=0.0, y=0.0):
        self.x = x
        self.y = y

class Velocity:
    def __init__(self, x=0.0, y=0.0):
        self.x = x
        self.y = y

class MovementProcessor(esper.Processor):
    def process(self):
        for ent, (vel, pos) in self.world.get_components(Velocity, Position):
            pos.x += vel.x
            pos.y += vel.y

world = esper.World()
world.add_processor(MovementProcessor())
player = world.create_entity(Velocity(x=0.9, y=1.2), Position(x=5, y=5))
world.process()  # repeat

The new API I suggest to write looks like this

import esper

class Position:
    def __init__(self, x=0.0, y=0.0):
        self.x = x
        self.y = y

class Velocity:
    def __init__(self, x=0.0, y=0.0):
        self.x = x
        self.y = y

def process_movement():
    for ent, (vel, pos) in esper.get_components(Velocity, Position):
        pos.x += vel.x
        pos.y += vel.y

esper.init_world()
esper.add_processor(process_movement)
player = esper.create_entity(Velocity(x=0.9, y=1.2), Position(x=5, y=5))
esper.process()  # repeat

Here is how I analyze the benefits and loss.

Readability

In process implementation, they are no unnecessary class X lines; they are no repeated self.world. The world instance is module-side, they are no world attribute in the App-level object or in global variables.

Performance

esper implementation become straightforward. Less calls to the dot operator in process implementation and even less using from esper import get_components. contextvar.get at the start of every esper function must have a performance cost. Need to write a benchmark to avoid taking a step back (first PR).

Modularity

It is harder to redefine World and Processor methods before use them to alter their behaviors. Because esper is a lightweight library, a developer with specific needs can implement a new function for this, so it is not so embarrassing.

Reusability

Using contextvars from the python standard library allows to use several World instances at the same time.

from contextvars import copy_context
import esper

def create_and_get():
    esper.create_entity(Position(0, 0))
    return list(esper.get_component(Position)

esper.init_world()
assert len(create_and_get()) == 1
cxt = copy_context()
cxt.run(exper.init_world)
assert len(cxt.run(create_and_get)) == 1
assert len(cxt.run(create_and_get)) == 2
assert len(create_and_get()) == 2

[benmoran56]

This is an interesting idea, and I'm interested in seeing the performance implications of switching to this approach. I'm sure there will be some improvement, for the reasons you've specified, but I wonder how much of the time is actually spent there as opposed to just iteration. It will be interested to see. I would suggest making an esper2, and comparing side-by-side by modifying the existing benchmarks.

The loss of easy modularity would be a big negative for me personally, since I tend to create one World per game scene. If the performance improvement is compelling, then we could try to find a user friendly solution. Perhaps a simple API taking advantage of contextvars I haven't used that module myself, but maybe we could do something like:

esper.init_world('world one')
esper.init_world('world two')

esper.switch_world('world one')

Also, it would be easy enough to allow World to accept functions/methods instead of classes in world.add_processor, but it feels like a half-measure, and not as usable having to pass the World instance around to different modules.

[Felecarpp]

I made a first draft here.

About implementation, I didn't use contextvars in the end. esper2.enter just reassigns esper2 functions from World methods. This way, so there's no performance gap from implementation. Some functions about processor management are rewritten simpler.

About performance implication, because World methods the same and are called directly, all the performance gap (out of esper2.enter call) is between statement resolve of the self.world.get_components and of the esper2.get_components. The score get from /examples/benchmark_api.py is fickle. I continue searching a way to improve performance and measure.

About the loss of easy modularity, World methods can be reimplemented by user as before. esper2.enter works like the esper.switch_world you suggested.

Therefore, the esper2 API now looks like this :

import esper2

class Position:
    def __init__(self, x=0.0, y=0.0):
        self.x = x
        self.y = y

class Velocity:
    def __init__(self, x=0.0, y=0.0):
        self.x = x
        self.y = y

def process_movement():
    for ent, (vel, pos) in esper2.get_components(Velocity, Position):
        pos.x += vel.x
        pos.y += vel.y

esper2.enter(esper2.World())
esper2.add_processor(process_movement)
player = esper2.create_entity(Velocity(x=0.9, y=1.2), Position(x=5, y=5))
esper2.process()  # repeat

[benmoran56]

I think the only way to get any performance difference is if you manually make all of the methods into module level functions. It shouldn't be too difficult - just remove self everywhere, and put the World attributes at the top as global variables.

_processors = []
_next_entity_id = 0
_components = {}
etc, etc

I was also thinking about context switching. There could be a default context (with some UUID). A context is really just a collection of the various data objects (as listed above). If a new context is made/switched to, the old context can be stored in a dict: {context_id: [_processors, _next_entity_id, etc, etc]}. Just some ideas.

[Felecarpp]

@benmoran56 The "Avoid global variables" injunction in the "Python performance tips" is really common. So I have little hope this way. However, all these articles seem to be cut-and-paste, then it's worth a try. https://duckduckgo.com/?q=python+peformance+tips

[benmoran56]

Yes, that's what I was thinking. It's worth a try just to confirm.

[Felecarpp]

Good News ! I get greet scores.

$ python -m examples.benchmark_api
esper1 query components, 2000 Entities: 5.735848 ms
esper2 query components, 2000 Entities: 5.481931 ms
score 22.635247453946775
$ python -m examples.benchmark_api
esper1 query components, 2000 Entities: 5.359582 ms
esper2 query components, 2000 Entities: 5.001317 ms
score 34.57849502145058
$ python -m examples.benchmark_api
esper1 query components, 2000 Entities: 5.480789 ms
esper2 query components, 2000 Entities: 5.162471 ms
score 29.907925480148688
$ python -m examples.benchmark_api
esper1 query components, 2000 Entities: 5.704820 ms
esper2 query components, 2000 Entities: 5.355262 ms
score 31.6053965630032
$ python -m examples.benchmark_api
esper1 query components, 2000 Entities: 5.422279 ms
esper2 query components, 2000 Entities: 5.155304 ms
score 25.239634343133325

And naturally

$ git diff --stat esper2.py
 esper2.py | 626 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------------------------
 1 file changed, 257 insertions(+), 369 deletions(-)

[benmoran56]

Those are compelling results. The numbers are not huge, but still valuable considering the scope of this library. How about making a new branch for this, and making a WIP pull request? (esper3_dev, or a similar name).

I also just noticed that the _event_registry can easily be part of the current context as well. That would be another bonus, since the current implementation is shared between Worlds.

[Felecarpp]

Those are compelling results. The numbers are not huge, but still valuable considering the scope of this library. How about making a new branch for this, and making a WIP pull request? (esper3_dev, or a similar name).

Ok, I will do this now.

I also just noticed that the _event_registry can easily be part of the current context as well. That would be another bonus, since the current implementation is shared between Worlds.

An event handler will already be world specific if the component type required by get_components do not exist in the actual world. Please, give me an use case where event handlers should not be shared between worlds ?

[Felecarpp]

Same answer about processors, why not shared processors between worlds ? We could also call them directly from the main loop. It should be simpler and faster. "Explicit is better than implicit."

[Felecarpp]

From the README.md

Avoid bloat as much as possible. New features will be considered if they are commonly useful. Generally speaking, we don't want to add functionality that is better handled in another module or library.

Considering this, I suggest remove Event system from esper due to the blinker library and the pubsub library.

[benmoran56]

Forgot about the branch limitation. I just made a new esper3 branch on my end, so please target that one.

An event handler will already be world specific if the component type required by get_components do not exist in the actual world. Please, give me an use case where event handlers should not be shared between worlds ?

Sure, event handlers don't necessarily have anything to do with Components. The main usage is usually Processors using events to signal things between themselves. For instance I have a MapProcessor that emits an event with the new map size. Other Processors, such as CollisionProcessor, and MovementProcessor, might be interested to know about the new map change.

Same answer about processors, why not shared processors between worlds ? We could also call them directly from the main loop. It should be simpler and faster. "Explicit is better than implicit."

I'm not sure what you're asking here. These two questions seem to be asking for opposite things. Could you elaborate?

From the README.md

Avoid bloat as much as possible. New features will be considered if they are commonly useful. Generally speaking, we don't want to add functionality that is better handled in another module or library.

Considering this, I suggest remove Event system from esper due to the blinker library and the pubsub library.

The Event system was added due to heavy demand. Many ECS libraries include something similar, so it was hard to argue against. Users can of course use another event library if they want, and are encouraged to do so if they want more functionality. I have personally started using the esper event system for my own projects as well, so I plan to keep it.

[Felecarpp]

Ok, I will push to this branch.

I use mainly the pure ECS functions from esper so I did not understand why these features. So the event system aims to remain minimal. It avoids to depend of additional libraries with superfluous complexity.

About event handlers and processor calls, their behavior already relies on context-dependent functions like esper.get_components. So I suggest to call them independently of the context.

In the case of processors, the simplest way to do this looks very good to me : direct calls to process functions in the main loop. So, processors can easily be grouped in functions. User can write processor specific arguments. It replaces the hard to maintains priority system by a readable suite of function calls.

[benmoran56]

I use mainly the pure ECS functions from esper so I did not understand why these features. So the event system aims to remain minimal. It avoids to depend of additional libraries with superfluous complexity.

About event handlers and processor calls, their behavior already relies on context-dependent functions like esper.get_components. So I suggest to call them independently of the context.

This is not necessarily always the case. Advanced types of processors may have different state, and handle events differently depending on how they are instantiated. For example, something like MyProcessor(scene_reference, cell_size, frequency). There may be a different instance of this processor in multiple Scenes in a game. Each instance of this Processor would get the same event, fired from a completely different Scene/World.

The current way around this is to manually remove event handlers when switching Scenes, but it's tedious. Making events part of the context will simplify this.

In the case of processors, the simplest way to do this looks very good to me : direct calls to process functions in the main loop. So, processors can easily be grouped in functions. User can write processor specific arguments. It replaces the hard to maintains priority system by a readable suite of function calls.

I can see the appeal of this, and of course the existing World.add_processor/process methods will not work when using plain functions as processors. However, I would like to keep esper.add_processor, esper.process, and the Processor base class for backwards compatibility. It will make the transition easier for existing projects, including my own.

[Felecarpp]

Thanks to replied to all these answers. I'm now writing a pull request.

[benmoran56]

Thank for all of your work on this. I merged in the pull request, and will test it with my own projects over the next few days.

There might be one or two small changes I would like to make, but overall I think it's looking pretty good.

[benmoran56]

I have made some changes & fixed some bugs, and have currently started testing with my personal projects. A few extra functions were needed to fit my workflow. So far it seems to be working as expected.

[elistevens]

Does the new API allow for having what would be two worlds active at the same time? I'm experimenting with having a world for the UI and a world for the game state. It's a turn based game, and it feels simpler to have the UI and game state tick (call process) at different rates. I worry that any performance gains from the proposed approach will be negated by having to switch contexts back and forth.

[Felecarpp]

Does the new API allow for having what would be two worlds active at the same time? I'm experimenting with having a world for the UI and a world for the game state. It's a turn based game, and it feels simpler to have the UI and game state tick (call process) at different rates. I worry that any performance gains from the proposed approach will be negated by having to switch contexts back and forth.

You are true. Having two contexts running at the same time with the new API requires at least two context_switch per loop turn. However, in a turn based game, the performance cost of the switch_context should not be penalizing.

I think it is simpler to implement GUI elements using classic class based programing than using ECS.

[benmoran56]

It would look something like this:

esper.switch_world('gui')
call_code()
esper.switch_world('myworld1')
call_code()

The performance impact of switching contexts should be negligible. (It's a function call, dict lookup, and less than a dozen attribute settings).

[benmoran56]

After lots of testing with my own projects with the alpha, I've release v3.0. I'll close this now as it's complete.