-
Notifications
You must be signed in to change notification settings - Fork 17
Add FlxGroup::revive()
#110
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
Add FlxGroup::revive()
#110
Conversation
The FlxGroup will now revive itself, and then all its members. This is to compliment `FlxGroup::kill()`.
|
Note that
|
|
Looks good to me. I agree about the order things should happen. |
org/flixel/FlxGroup.as
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be && !basic.exists).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Damned my copy-pasting!
But after some thinking, I'm still a bit unclear as to the difference between exists and alive in this context. They seem to be used interchangeably in the code, with the only difference being is draw() is only run if the object exists.
You can read the following in the ASDoc of FlxBasic::kill():
Default behavior is to flag them as nonexistent AND dead. However, if you want the "corpse" to remain in the game, like to animate an effect or whatever, you should override this, setting only alive to false, and leaving exists true.
If the user has done this override on the objects, would it be better to only resurrect the object if it is not alive? Or perhaps check both fields?
if((basic != null) && (!basic.alive || !basic.exists))Or would the user override the FlxGroup as well to make sure it revives all objects, even ones that are dead but still exist? (this seems unnecessary) Is it easier to always run basic.revive(), and also add a line to FlxBasic::revive() that only revives the object if it is dead? Or maybe if it does not exist? (subclasses would need to make sure they implement this as well)
I think I'm overcomplicating things here. Someone take a step back, look at the class, and let me know what to do. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always thought exists and alive are something odd about Flixel. Analyzing FlxGroup::kill(), it checks if the object exists, so I think we should do the same. In that case, all you have to do is add the missing ! to your code.
If we dig too deep, we will end up with all those questions you raised. I think it's enough to test if the object exists before calling revive(). It's something like "if you exist, go back to life; if you are already alive, deal with it yourself" 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest that revive only resurrects objects that are not alive.
if ((basic != null) && !basic.alive)It's fairly common to override kill to set an objects alive to false but to keep exists true, so that you can leave dead bodies on the screen for example. Having FlxGroup::revive like this would make it easier to revive such objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds a good replacement. I never thought about "corpses" in the game :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, I like the alive check.
I made the changes and pushed the commits. Any further changes, or does everything look good now?
It's a minor typo, but with major consequences! If left unchecked, this bug would revive already existing objects, leading to several Science Fiction worthy paradoxes. Also fixed a tiny spelling typo in the ASDoc.
After some discussion on GitHub https://github.com/FlixelCommunity/flixel/pull/110/files#r1831839 it seems the more logical to check if an object is `alive`, rather than check if it `exists` when reviving it.
|
The issues mentioned by @moly in the previous commits have now been fixed. Does everything look good now? |
|
It's good to me. |
The following commit has been undone: FlixelCommunity#110 And a tiny bit of documentation has been added describing the changes. Discussion: - FlixelCommunity#185 - FlixelCommunity#30 (comment)
The FlxGroup will now revive itself, and then all its members.
This is to compliment
FlxGroup::kill().Resolves the following issue:
#30