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

FlxGroup: implement revive() #1243

Merged
merged 1 commit into from
Jul 27, 2014
Merged

Conversation

shinji-yoshida
Copy link
Contributor

FlxTypedGroup has method kill() that kills all members in group, but revive() did not revive them.
I have implemented revive() so that all members to be revived.
Please merge.

@Gama11
Copy link
Member

Gama11 commented Jul 25, 2014

I think this might cause issues with FlxEmitter that calls revive() in start().

@shinji-yoshida
Copy link
Contributor Author

Thank you for pointing out.
I have added reviveOnlyGroup() to FlxTypedGroup and use it in FlxEmitter#start().

@Gama11
Copy link
Member

Gama11 commented Jul 26, 2014

I'm not really a fan of this solution, tbh, but I can't really think of a better one either since haxe has no function overloading...

@shinji-yoshida
Copy link
Contributor Author

I pushed another solution. What do you think?

@Gama11
Copy link
Member

Gama11 commented Jul 27, 2014

To be honest, I think it's quite a mess and way too implicit (I would prefer an explicit solution like adding a reviveMembers param to the function, but since you can't change the function signature, that's out of the question). Something this simple shouldn't require this much code.

Now, I do agree that revive() should probably revive all its members to be consistent with kill() - anything else seems counter-intuitive. I haven't checked, but it's probably fine if we replace the revive() call in FlxEmitter by exists = true?

@shinji-yoshida
Copy link
Contributor Author

It seems that I had clung to calling revive() too much.
Replacing revive() by exists = true is much better!

I have replaced revive() by exists = true and rebased to latest dev.
Please check it.

Gama11 added a commit that referenced this pull request Jul 27, 2014
@Gama11 Gama11 merged commit 177967e into HaxeFlixel:dev Jul 27, 2014
@shinji-yoshida shinji-yoshida deleted the flx_group_revive branch July 28, 2014 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants