-
Notifications
You must be signed in to change notification settings - Fork 124
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
Refactored Rubble to use ECS #593
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Most of my comments are small naming/documentation things, other than the one issue about freeing the Vector too early.
I have not been able to test it (still don't have my workspace set up well), so I haven't been able to see the physics issues. One that I'm noticing (that could be a relevant factor) is that there isn't a good way to set the Velocity, Angle, AngularVelocity, or Position after the entity has already been created. Fixing that is outside of the scope of this PR, of course, but your comment about AngularVelocity made me realize the issue. I'll fix that at some point.
engine/src/main/java/org/destinationsol/rubble/components/RubbleMesh.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/destinationsol/rubble/systems/RubbleCreationSystem.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/destinationsol/rubble/systems/RubbleCreationSystem.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/destinationsol/rubble/systems/RubbleCreationSystem.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/destinationsol/rubble/systems/RubbleCreationSystem.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/destinationsol/rubble/systems/RubbleCreationSystem.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/destinationsol/rubble/systems/RubbleCreationSystem.java
Outdated
Show resolved
Hide resolved
engine/src/main/java/org/destinationsol/rubble/systems/RubbleCreationSystem.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! @NicholasBatesNZ can you review/merge this?
Tested this out real quick, after a trivial conflict fix. Can confirm rubble comes out of the golden asteroid, but ... the rubble is invincible? 😅 I can tell I'm hitting it, physics shape seems alright but since so small it is hard to be sure. Also, should the golden asteroid break into incrementally smaller pieces, rather than go to the tiniest size immediately, or is that outside the scope of this PR? Since that asteroid is a test piece anyway. |
Oh, and the invincible rubble seems to be pre-existing specific to the stuff the golden asteroid pops out. Conflict resolution was just accepting all from this PR then adjusting three "shard" to "rubble" |
Merged this after resolving the conflict locally. Submitted #617 for follow-up fixes. Since it is a test asteroid it doesn't exactly have to be perfect - yet 👍 |
This pull request refactors rubble to use the Entity Component System architecture. Currently, only rubble spawned from the test asteroid at spawn will be created using ECS, as that asteroid is the only entity with the CreatesRubbleOnDestruction component.
Outstanding Issues
There are some issues concerning the physics of the rubble, as it seems there is not a way to add angular velocity with the velocity component yet. The textures also appears to be offset slightly from the collision mesh.