Added AttachmentSystems + docs + example #245
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sorry for Wall of text*
What does this PR do?
setup.py
script to support c++.New GameSystems:
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.
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.
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 thekivent_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
Potential bugs:
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
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.
_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.