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

Inconsistent Entity Hierarchy #336

Closed
nightm4re94 opened this issue Apr 11, 2020 · 1 comment
Closed

Inconsistent Entity Hierarchy #336

nightm4re94 opened this issue Apr 11, 2020 · 1 comment

Comments

@nightm4re94
Copy link
Member

nightm4re94 commented Apr 11, 2020

Have a look at this excerpt of our entity hierarchy:

├Entity (implements IEntity)
|  ├CollisionEntity (implements ICollisionEntity)
|  |  ├...
|  |  ├CombatEntity (implements ICombatEntity)
|  |  |  ├Creature (implements IMobileEntity)
|  |  |  ├Prop
|  |  ├MobileEntity (implements IMobileEntity)
...

MobileEntity and Creature both implement IMobileEntity and as there is currently no use for MobileEntites that are not CombatEntities, we have some redundancy going on. Do we actually need the MobileEntity implementation? Sure, one might come up with scenarios where you want something to move but not take part in combat, but in that case there are ways to exclude CombatEntities from combat anyway.
I think we could ditch MobileEntities entirely for the sake of simplicity and not confusing developers too much.

@steffen-wilke
Copy link
Contributor

That's just the downside of an inheritance-based Entity hierarchy compared to a component-based architecture. I think however our interfaces kind of define the "components" an entity can provide.

Ultimately this comes down to: potentially some runtime memory wasted vs. some duplicated code. I don't think either downside is really significant. But I would agree with the argument of trying to make the structure as simple as possible, i.e. ditching the MobileEntity implementation and just providing Prop and Creature is a much cleaner concept.

steffen-wilke pushed a commit that referenced this issue Apr 14, 2020
This closes #336, where we have decided to ditch the approach of having two separate IMobileEntity implementations.
From now on, Creatures are to be used instead of MobileEntities.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants