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

Added AttachmentSystems + docs + example #245

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SleepProgger
Copy link
Contributor

Sorry for Wall of text*

What does this PR do?

  • Adding 3 game systems to support local coordinates and entity relationships.
  • Adding an example on how to use the local coordinate system.
  • Updating the docs to reflect the new GameSystems.
  • Updating the core setup.py script to support c++.

New GameSystems:

  • RelationTreeSystem:
    This is the base class for the LocalPositionSystem2D.
    It allows to construct relation ship trees and ensures that references to parent systems are removed when the parent is removed.
  • LocalPositionSystem2D:
    Extends the RelationTreeSystem and implements a local coordinate system (no rotation).
    This is done by using the GameSystems ZonedAggregator to hold pointer to the parents components. Each entity has to own a PositionComponent2D for its local, global and parent coordinates.
    The LocalPositionSystem2D fills a c++ vector with all entities in the "level-order" to be able to iterate over this list to update the positions in the correct order (parent always before child).
    This makes it possible to cache the iteration order and allows this class to be easily extended (in cython) for other systems with similar needs.
  • LocalPositionRotateSystem2D
    Extends the LocalPositionSystem2D and adds support for rotation.

Changes in setup.py

Because of the use of c++ container some updates in the setup.py script of the kivent_core module where necessary. It should now be able to properly build C and C++ code, but i am not sure if different platforms may need parameter updates for the compiler. Thus i'd propose to NOT merge this PR with the master branch yet, but to create a test branch and test it properly on different platforms first.
IMHO this is a good test as some of the existing kivent classes would also benefit from a change to c++ stl container.
It works without a problem on my debian stretch system.

Docs

I did not generate the documentation for every method but decided to only list the "relevant" ones including some inherited methods. IMHO the user doesn't care about the overwritten allocate method f.e.
Additionally i added the :show-inheritance: argument to display the parent classes. (This might be a good idea to do for every other documented class btw).
The docs in html form

Example

enter image description here

Potential bugs:

  • If a parent component (PositionComponent or RotationComponent) is removed by calling remove_component on the corresponding GameSystem the LocalPositionSystem2D would use a non active or wrong component. We could check the entity_id of every parent component for every entity in the LocalPositionSystem2D update loop to raise an Exception in that case. But i don't think its worth the resulting performance hit. Every other GameSystem using a ZonedAggregator has the same problem.

Things to do / i am not sure about / Questions

  • Every RelationComponent which owns children has a c++ set to hold its children.
    It might be a good idea to change it to a vector (because of the memory footprint) and use a custom memory allocator so they'd use the static memory, but i am not sure if its worth it.
  • The c++ vector buffering the iteration order could be changed to use an custom memory allocator to use the static memory or could possible be replaced by allocating an additional MemoryZone. I'd prefer the vector variant. Again: not sure if its worth it as the memory footprint of this is relative small (1 pointer per used non root entity vs. 1 pointer for every entity in the GameSystems zones).
  • The RelationTreeSystem holds a _state variable containing a wrap around counter which is increased after every change to the tree. This allows to detect if the iteration order buffer needs to be updated. In theory this could lead to a bug where exactly max_value(unsigned int) changes since the last update wouldn't be detected.
    It might be wise to change this to a boolean which is reseted after every update loop but that would mean we can only use this information in the class extending the RelationTreeSystem and not the python side fe. After all i think that is a change that should be done, but i am not sure if keeping the _state as additional variable might make sense.
  • I am not sure about the naming of the systems. If someone has a better idea please share.
  • The kivy part of the example might be a bit more complicated as it has to be as i am not that familar with kivy yet. If someone knows how to do the same in less and easier code please feel free to share.

@SleepProgger
Copy link
Contributor Author

SleepProgger commented Jul 16, 2017

Forgot one thing:
I copied the LICENSE file for the example, so it contains Kovaks name.
I don't really like to change it to my real name. Not sure how to handle this.

BTW: all the licenses contain a duplicated line (16 and 17)

@Kovak
Copy link
Contributor

Kovak commented Aug 8, 2017

I need to use this one but it looks beautiful. You can also put a handle such as SleepProgger in there if you want some credit but not reveal your identity.

@SleepProgger
Copy link
Contributor Author

Happy to hear that.
I will change the license and fix a bug in the example the coming days.

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

Successfully merging this pull request may close these issues.

2 participants