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

[Discussion] Sectors Implemenation/API #2976

Closed
Vizaxo opened this issue Jun 14, 2017 · 7 comments
Closed

[Discussion] Sectors Implemenation/API #2976

Vizaxo opened this issue Jun 14, 2017 · 7 comments
Labels
Category: Performance Requests, Issues and Changes targeting performance Multiplayer Affects aspects not visible in Singleplayer mode only Status: Needs Investigation Requires to be debugged or checked for feasibility, etc. Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc.

Comments

@Vizaxo
Copy link
Contributor

Vizaxo commented Jun 14, 2017

Sectors are a concept I am implementing into Terasology as part of my GSoC project. They have been described before (forum, github), but here's a basic rundown.

Basic explanation

The current entity system stores entities in a global scope. Sectors introduce multiple different entity caches, each of which stores its own entities and components. This allows multiple different sectors to be present in the world, split up in a way that means entities usually only interact with other entities in their own sector (e.g. two sectors on different continents).

Currently my implementation splits the EntityManager into EntityManager and EntityCache. The new EntityCache is for the storage and creation of entities, and the EntityManager contains multiple EntityCaches (one for the global store, and one for each sector (currently there is only a single sector)). The EntityManager methods still work, and they operate on the global cache by default.

Questions

There are some questions to answer about how they will be implemented, and how the API can be designed to communicate with the sectors. This thread is to get some input on how sectors should be built (both for now and the future), so any feedback is welcome. I'll keep updating it as more questions/possibilities arise.

How transparent should sectors be? Should classes that create entities be able to choose which sector to put the entities in?

I'm leaning towards having classes that do entity creation/access decide between global- or sector-scope, and a sector manager will handle which sector to interact with, if there are multiple to choose from.

What should the API look like?

Currently, the EntityManager has many methods to create and interact with entities and components. My current implementation moves most of these to the EntityCache (so it is easy to create entities in the global scope, or in any sector), with the EntityManager versions just calling the ones in the global cache. Should there be separate methods to create an entity in sector-scope, or should you be able to access the sector-level cache and create entities on it directly?

@skaldarnar
Copy link
Member

Also consider #2975 for this discussion.

Some of my comments on your PR address some of the questions here. If the user has the responsibility (or at least possibility) to create and assign sectors the interfaces might become easier/more accessible. However, this goes against the idea of having sectors automatically split (depending on some properties still to be determined).

Only differentiating sector and global scope might also fall short of benefits in the long run. In the end, it just introduces a single additional entity store - this could probably also be modelled in a different way using the current manager and the global store only.

Asking for some more input from our architects here to get some input!

@Vizaxo
Copy link
Contributor Author

Vizaxo commented Jun 16, 2017

@skaldarnar (this reply will cover both the comment above and the PR review here)


Only differentiating sector and global scope might also fall short of benefits in the long run. In the end, it just introduces a single additional entity store - this could probably also be modelled in a different way using the current manager and the global store only.

there is only one other entity cache, and entities live either in one or the other;

The current implementation only has one sector-level entity cache, and I agree that that might not provide many benefits. The plan is for the sector manager to allow handling of multiple caches, so there can be many sectors in a world.

If all sector-level interactions go through the sector manager, it can decide to split/merge/move sectors in the way that best fits the situation.


If the user has the responsibility (or at least possibility) to create and assign sectors the interfaces might become easier/more accessible. However, this goes against the idea of having sectors automatically split (depending on some properties still to be determined).

Yeah, if the user can manually assign/manipulate sectors, it makes it harder for the sector manager to split or move sectors. I wonder if the user even needs to know which sector the entity is in; one option is to to completely encapsulate the sectors inside the entity manager and sector manager. They might need to expose some details, like finding out if two entities are in the same sector, but I think for the most part it's easier and there's less to worry about if the user doesn't have a say in which sector the entity goes into, just whether it goes into sector- or global-scope.

I'm wondering whether we can parameterize the existing methods with some kind of sector identifier (SectorRef) - method overloading should allow to keep it backwards compatible? Alternatively, the sector ref parameter could be an Optional<SectorRef> where Option.empty() defines that the entity is created in global scope...

I think something like this would be a good option if we do allow the user to directly choose the sector. But if we only allow the user to determine the scope, and not the specific sector, a parameter for the scope seems like it would work (and more scopes could be added later, such as a world-scope, for when multi-world games are introduced). Alternatively, a new set of methods (e.g. createSectorEntity) could exist alongside the create methods, but that might be more confusing, and be less flexible in the future.


I'm leaning towards hiding the details of specific sectors, and having the sector manager manage it as much as possible. I can't really see a use-case where the added complexity of having the user track the sector id/ref would provide a benefit (but I'm all ears if someone can think of benefits of it). This way also gives more power to the sector manager to manage as it sees fit.

@Vizaxo
Copy link
Contributor Author

Vizaxo commented Jun 24, 2017

I've written up some notes on the current API of the entity manager (here). I've copied the notes section here, which outlines some of the weird/inconsistent methods that are probably worth renaming/consolidating.

Creation

  • long createEntity()

    The name isn't immediately obvious (it generates an id that can be used to create an entity, not a new entity); perhaps something like generateNewId would be better?
    
  • createEntityRef(long entityId), createEntityRefWithId(long entityId), createEntityWithId

    createEntityRef is used by lots of the internal functions to do the entity creation
    
    createEntityRefWithId only used in EntityRefTypeHandler
    
    createEntityWithId is used in network/serialization
    
    It seems weird that there are so many different functions with similar names
    
    I think they should be renamed to be the same as create(), so it is obvoious what they do
    
  • createEnittyWithoutLifecycleEvents could just be a flag on create()?

Destruction/removal

  • destroyEntityWithoutEvents could just be a flag on destroy

  • It seems weird that destroy operates on ids, but WithoutEvents operates on refs

  • There is a private destroy that works on refs (but doesn't send events)

Components

  • addComponent and saveComponent are a bit confusing

    addComponent will log an error if it overwrites a component
    
    saveComponent will log an error if it doesn't overwrite a component
    

Other

  • getEntity will create a new entity if one doesn't exist for that id

    From the name, it looks like it should just retrieve an entity if it exists, or return EntityRef.NULL otherwise
    

@Cervator
Copy link
Member

Alright, finally had some time to look at this, and we talked a few days ago. Feels like we're slowly narrowing in on something here :-)

Some comments:

  • Sectors should be very ghostly - extremely transparent and flexible, only relevant in the deep back-end of the game engine. Sectors don't get stored to disk or anything like that - they are just virtual containers useful at runtime.
  • IMHO no content or logic should ever target a specific sector, outside of the sector management itself (when to split into two, or merge if border activity is high). Possible point of confusion: in multi-world you might be targeting specific worlds for entities. Both these items (dynamically splitting/joining sectors and multi-world) may be out of scope for GSOC. Maybe multi-world is similar to sectors, just more rigid (harder to float between them, permanent)
  • Content could offer advice to the engine for how to process it at sector scope if the content should remain loaded and simulating even without nearby chunks loaded (examples: city or river simulation)
  • Sectors (groups of entities) and Sector Scope (processing entities without loaded chunks) might actually be considered distinct (but related) concepts ... maybe need better naming
  • Global entities are except from anything Sector related - they are always loaded and active at whatever they do. Goal should be to minimize global entities

On long createEntity() - I suspect (@immortius could confirm) that this method is actually named correctly as an entity fundamentally is simply the number.

All the magic in the entity system is attaching components to a given entity, using an EntityRef - it isn't like other systems where there is an Entity class of some sort

Still, with gestalt-entity-system now properly released at v6.0.0 (the earlier v6 was missing some stuff, whoops), probably we should refocus on its architecture, which may differ

To this day I'm still not sure if sectors should be supported in Gestalt itself or in the game using Gestalt - made a bit trickier still by current engine having a "legacy" ES embedded. Probably would be good to try catching a bit of time from Immortius on the topic sometime and poke at the potential architecture some more :-)

@Cervator Cervator added Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc. Multiplayer Affects aspects not visible in Singleplayer mode only Category: Performance Requests, Issues and Changes targeting performance Status: Needs Investigation Requires to be debugged or checked for feasibility, etc. labels Jun 25, 2017
@vampcat
Copy link
Contributor

vampcat commented Jun 26, 2017

@Cervator In Gestalt-ES 6.0, createEntity returns an EntityRef :D
That being said, changing the way this works in current Terasology-ES might be too large, and it might be better to wait till we can integrate Gestalt-ES into Terasology (hopefully sometime soon after GSoC, once we've run some stress tests on the library in DestSol and optimized it).

As for the sectors in Gestalt - this is just my personal opinion, but since sectors are specific to Terasology, it might be better to implement them in Terasology using ES (like we're doing now).

@Vizaxo
Copy link
Contributor Author

Vizaxo commented Jun 27, 2017

@Cervator:

Sectors should be very ghostly - extremely transparent and flexible, only relevant in the deep back-end of the game engine. Sectors don't get stored to disk or anything like that - they are just virtual containers useful at runtime.

Noted; that's what I'm going for at the moment. I am currently working on storing the scope of the entity to disk, but as mentioned below, that's a distinct concept from the actual cache the entity is in.

Sectors (groups of entities) and Sector Scope (processing entities without loaded chunks) might actually be considered distinct (but related) concepts ... maybe need better naming

Yeah, I think so. Currently I'm using 'Cache' for a collection of entities. So all of the global-scoped entities are in one cache, and all of the sector-scoped entities are in another cache (which could be broken down further in the future).

On long createEntity() - I suspect (@immortius could confirm) that this method is actually named correctly as an entity fundamentally is simply the number.

Yeah, that is true; it just seems a bit confusing alongside all of the other create...() methods which return refs. It's currently only used internally inside the PojoEntityManager to generate the ids for the ref-returning methods.


@vampcat:

@Cervator In Gestalt-ES 6.0, createEntity returns an EntityRef

It looks like that's replaced the create() methods that TS uses (the EntityRef-returning ones) as well, so it's already done the method consolidation I was talking about.


Still, with gestalt-entity-system now properly released at v6.0.0 (the earlier v6 was missing some stuff, whoops), probably we should refocus on its architecture, which may differ
To this day I'm still not sure if sectors should be supported in Gestalt itself or in the game using Gestalt - made a bit trickier still by current engine having a "legacy" ES embedded. Probably would be good to try catching a bit of time from Immortius on the topic sometime and poke at the potential architecture some more :-)

As for the sectors in Gestalt - this is just my personal opinion, but since sectors are specific to Terasology, it might be better to implement them in Terasology using ES (like we're doing now).

I'm not sure about this one either. I can see a possibility for multiple caches being useful for other games (especially games which look to scale to big multiplayer games), without actually saying anything about how the caches are used (but I agree that the idea of sectors is specific to TS). Caches are currently implemented so that the EntityManager is a cache, so they will effectively stay hidden if the game doesn't need them, but multiple caches could be implemented in each game as it sees fit.

@syntaxi
Copy link

syntaxi commented Feb 3, 2018

@Vizaxo @Cervator Can this now be closed what with the existence of #3144 and the merging of #3009. I think it can and possibly even have some of the outline here spun into documentation (or simply added/duplicated to the forum)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Performance Requests, Issues and Changes targeting performance Multiplayer Affects aspects not visible in Singleplayer mode only Status: Needs Investigation Requires to be debugged or checked for feasibility, etc. Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc.
Projects
None yet
Development

No branches or pull requests