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

Signals fix #52

Merged
merged 2 commits into from
Aug 30, 2014
Merged

Signals fix #52

merged 2 commits into from
Aug 30, 2014

Conversation

vlaaad
Copy link
Contributor

@vlaaad vlaaad commented Aug 28, 2014

Hi there!
First of all, ashley is a very nice framework (may be too small), and I like to play with it :)
Although, it's not ideal. I propose a change to Signal, so it will correctly handle cases of modification of listeners list while dispatching is being in progress.
There are 2 commits:

  1. First adds a test case, which fails on current Signal implementation. Signal should notify every listener during dispatch, but if listener is removed when notified (which can happen very often), the next listener is skipped.
  2. Second fixes failed test case :) Solution uses SnapshotArray, which freezes array during dispatch. I looked at it's source and it seems it gives very little overhead.

What do you think?

@dsaltares dsaltares added the bug label Aug 29, 2014
@@ -1,12 +1,12 @@
/*******************************************************************************
* Copyright 2014 See AUTHORS file.
*
Copy link
Member

Choose a reason for hiding this comment

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

Can you please revert this lines?

@dsaltares
Copy link
Member

Thanks! Other than the small comment, it looks great. Much appreciated. Can you please commit the fixes and rebase to have a clean history?

@vlaaad
Copy link
Contributor Author

vlaaad commented Aug 29, 2014

Here it is, I fixed comments and made it a single commit.

@vlaaad
Copy link
Contributor Author

vlaaad commented Aug 29, 2014

Oh.. Looks like I introduced another stupid changes ><
Do you have some kind of eclipse formatter (like libgdx does), so I can use it to keep project's code style and formatting?

@dsaltares
Copy link
Member

It's alright, you removed some extra spaces anyway! Thanks :-).

dsaltares added a commit that referenced this pull request Aug 30, 2014
Fixes Signal dispatching bug while deleting
@dsaltares dsaltares merged commit a07576f into libgdx:master Aug 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants