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.

Sign up for GitHub

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

Jump to bottom

Create a basic ECS #17

Merged
merged 21 commits into from
Apr 8, 2022
Merged

Create a basic ECS #17

merged 21 commits into from
Apr 8, 2022

Conversation

foodelevator
Copy link
Collaborator

@foodelevator foodelevator commented Mar 15, 2022 •
edited
Loading

First parts before closing #5

Features to implement before merging this PR into main

  • Keep track of entities
  • Store components
  • Query for (multiple) components
  • Iterate over all entities matching a query
  • Resources

@foodelevator foodelevator self-assigned this Mar 15, 2022
@foodelevator foodelevator changed the title Create an ECS Create a basic ECS Mar 16, 2022
@ollisco
Copy link
Collaborator

ollisco commented Mar 16, 2022

We could maybe split up workflows so that everything is not in the same file.

ollisco
ollisco reviewed Mar 16, 2022
View reviewed changes
Copy link
Collaborator

@ollisco ollisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great just a few comments on what could be improved slightly.

Some documentation of the more complex features and on the tests.

ecs/src/lib.rs Show resolved Hide resolved
ecs/src/component/registry.rs Show resolved Hide resolved
@ollisco ollisco added the ECS🏴󠁥󠁣󠁳󠁿 Relatet to the Entity Compnent System label Mar 16, 2022
@foodelevator
Fix some deadly bugs 🪲; Cleanup code; Improve doc
01c8466
@foodelevator
Remove the EntityBuilder
5e0fc23 
The engine itself won't add that many components and this struct cant be
used from a potential scripting language, so it didn't make any sence to
add from the beginning.
@foodelevator
Add component *queries* and some validation
c14cd9e
@foodelevator
Add documentation & make clippy 📎 happy
b7f4b5d
@foodelevator
Begin creating query response (NOT WORKING)
3aef170
@foodelevator foodelevator linked an issue Mar 18, 2022 that may be closed by this pull request
Create an ECS #5
Open
22 tasks
@foodelevator foodelevator removed a link to an issue Mar 18, 2022
Create an ECS #5
Open
22 tasks
@foodelevator foodelevator mentioned this pull request Mar 18, 2022
Closed
@foodelevator
Copy link
Collaborator Author

foodelevator commented Mar 18, 2022 •
edited
Loading

Retrieving data is pretty unergonomic and requires unsafe code (since what you get is raw pointers).

I think the general solution would require proc macros and I don't think that's worth the hassle TBF.

The compile time type safety would still not be available when interacting with a scripting language and it's pretty easy to write wrapper functions that handle specific queries in a type safe way.

@foodelevator foodelevator marked this pull request as ready for review March 20, 2022 15:43
ollisco
ollisco previously approved these changes Apr 5, 2022
View reviewed changes
Copy link
Collaborator

@ollisco ollisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I agree that we don't have to prioritise removing unsafe even though it would be nice :)

ghagl
ghagl previously approved these changes Apr 8, 2022
View reviewed changes
Copy link
Member

@ghagl ghagl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ser bra ut. Härnäst blir det att fundera över föräldra-barn-relationer (d.v.s. om ett svärd finns hos en karaktär, tas karaktären bort så ska svärdet bort)

@ollisco
Copy link
Collaborator

ollisco commented Apr 8, 2022

@mathiasmagnusson Kan du fixa dessa merge conflicts?

@foodelevator
Copy link
Collaborator Author

foodelevator commented Apr 8, 2022 •
edited
Loading

Yes jag fixar det idag

@foodelevator foodelevator dismissed stale reviews from ghagl and ollisco via 2dc23df April 8, 2022 14:25
@foodelevator
Copy link
Collaborator Author

Seems like we need those reviews again...
The only change is that i merged changes from the main branch

ollisco
ollisco approved these changes Apr 8, 2022
View reviewed changes
Copy link
Collaborator

@ollisco ollisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@foodelevator foodelevator merged commit a6d5eba into main Apr 8, 2022
@foodelevator foodelevator deleted the mathm/ecs branch April 8, 2022 18:41
@foodelevator foodelevator restored the mathm/ecs branch April 18, 2022 10:08
@foodelevator foodelevator deleted the mathm/ecs branch April 18, 2022 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ollisco ollisco ollisco approved these changes

@ghagl ghagl ghagl approved these changes

@RealButterbee RealButterbee Awaiting requested review from RealButterbee

@lukasfri lukasfri Awaiting requested review from lukasfri

@OlleStrand OlleStrand Awaiting requested review from OlleStrand

@TerraDOOM TerraDOOM Awaiting requested review from TerraDOOM

@VincentLagerros VincentLagerros Awaiting requested review from VincentLagerros

@VnomArrow VnomArrow Awaiting requested review from VnomArrow

Assignees

@foodelevator foodelevator

Labels
ECS🏴󠁥󠁣󠁳󠁿 Relatet to the Entity Compnent System
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@foodelevator @ollisco @ghagl