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

:sparkles: Destroy nested copies #127

Merged

Conversation

@schadocalex
Copy link
Contributor

schadocalex commented Oct 2, 2019

Changes proposed in this pull request:

  • When a copy is destroyed, all its children are destroyed too.
  • Use filter in place instead of multiple splice()

Ping @CosmoMyzrailGorynych

@schadocalex schadocalex changed the title :sparkles: Destroy nested copies :WIP: :sparkles: Destroy nested copies Oct 2, 2019
@schadocalex schadocalex changed the title :WIP: :sparkles: Destroy nested copies WIP :sparkles: Destroy nested copies Oct 2, 2019
@CosmoMyzrailGorynych

This comment has been minimized.

Copy link
Collaborator

CosmoMyzrailGorynych commented Oct 2, 2019

What does ct.types.onDestroy.apply(ct.stack[i]); do?

All the ct.types.on* methods are populated by modules through injects, and thus these include logic common to all copies' types.

@CosmoMyzrailGorynych CosmoMyzrailGorynych added this to In progress in Main dev board Oct 2, 2019
app/data/ct.release/main.js Outdated Show resolved Hide resolved
app/data/ct.release/main.js Outdated Show resolved Hide resolved
app/data/ct.release/main.js Outdated Show resolved Hide resolved
@schadocalex schadocalex changed the title WIP :sparkles: Destroy nested copies :sparkles: Destroy nested copies Oct 2, 2019
@schadocalex schadocalex requested a review from CosmoMyzrailGorynych Oct 2, 2019
@CosmoMyzrailGorynych CosmoMyzrailGorynych mentioned this pull request Oct 3, 2019
1 of 9 tasks complete
@CosmoMyzrailGorynych

This comment has been minimized.

Copy link
Collaborator

CosmoMyzrailGorynych commented Oct 3, 2019

It throws an error when a parent is deleted :/ Children can be deleted safely.
изображение

I made a number of tiny test cases, it might help to debug:
n127.zip

@CosmoMyzrailGorynych CosmoMyzrailGorynych changed the title :sparkles: Destroy nested copies WIP :sparkles: Destroy nested copies Oct 3, 2019
@schadocalex

This comment has been minimized.

Copy link
Contributor Author

schadocalex commented Oct 4, 2019

Thanks for the test cases! The last one was broken, I have repared it: n127.zip

I use a totally different approch for killing: I let pixi kill children and just check after for killed copies.

@schadocalex schadocalex changed the title WIP :sparkles: Destroy nested copies :sparkles: Destroy nested copies Oct 4, 2019
@CosmoMyzrailGorynych

This comment has been minimized.

Copy link
Collaborator

CosmoMyzrailGorynych commented Oct 4, 2019

Well, yes, I began to confuse the first and second containers and mixed up the indices 😅

But I've found another bug ¯_(ツ)_/¯ It is dependant on the creation order (more precisely on the position of a copy in the stack). See Test_KillTwo and Test_KillTwo_reverse rooms.

n127.zip

I suspect it is because pixi freaks out when one tries to destroy an already destroyed DisplayObject. Idk why pixi doesn't use its own variable to catch these, but a check for copy._destroyed may be enough.

Copy link
Collaborator

CosmoMyzrailGorynych left a comment

🎉

@CosmoMyzrailGorynych CosmoMyzrailGorynych merged commit 9be57f5 into ct-js:develop Oct 6, 2019
2 checks passed
2 checks passed
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Main dev board automation moved this from In progress to Done Oct 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.