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

#129 reverse system dispose and entity onRemove order. #130

Merged

Conversation

metaphore
Copy link
Contributor

Follow up for #129

  • Call system dispose in reverse order
  • Call FamilyOnRemove.onRemove(entity: Entity) in reverse order (depends on the bound system).
  • Dedicated tests for system execution order.

I'm not sure what's the best way to do a reverse "for" loop on an array in Kotlin. Sounds like there's no built-in singular function for that, and I didn't want to use something that may create an iterator or do unnecessary allocation like collection.asReversed().forEach(). So I added an inline extension function forEachReversed() at the end of the world.kt file. Don't know if there's any better place.

@Quillraven please let me know if there's something wrong with it and if it needs to be changed/moved.

System order tests.
@Quillraven
Copy link
Owner

You are correct that there is no build in function. The only one that I found is:

for (i in array.indices.reversed()) {
    println(array[i])
}

According to bytecode it looks like this is a simple for loop without iterator or other objects.

I personally would just use that without a separate extension function but your way is also fine.

@Quillraven Quillraven merged commit ed9f4d4 into Quillraven:master Dec 2, 2023
4 checks passed
@Quillraven Quillraven added this to the 2.6 milestone Dec 2, 2023
@Quillraven Quillraven added the enhancement New feature or request label Dec 2, 2023
@metaphore
Copy link
Contributor Author

I forgot, but it would be good also to update the FamilyOnAdd.onAdd() and IntervalSystem.onDispose() docs to reflect the reverse order of those functions.

@metaphore metaphore deleted the feature/reverse-remove-order branch December 3, 2023 18:34
@Quillraven Quillraven mentioned this pull request Dec 12, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants