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

"Complete all timers" and "complete all tweens" helper functions #1782

Merged
merged 19 commits into from
Aug 21, 2016

Conversation

IBwWG
Copy link
Contributor

@IBwWG IBwWG commented Mar 18, 2016

No description provided.

GimmickyApps added 3 commits February 23, 2016 08:14
Catch up with upstream.
Catch up with upstream.
Also, add a note to the docs.  Personally I found reading the FlxTimerManager API doc rather unenlightening and only found out how to use it by grepping the flixel source tree.

The completeAll() function I found useful for my project, where I had several dozen timers going with durations set like "i * 0.05" in a loop.  When I wanted to have it so the user could press a key to skip to the end of the sequence thus created, completeAll() made this a snap.
@IBwWG
Copy link
Contributor Author

IBwWG commented Mar 18, 2016

Actually, I'd like this for FlxTween too. There, done. (Maybe a sound.play callback one would be handy too, some day.)

@MSGhero
Copy link
Member

MSGhero commented Mar 18, 2016

Are pauseAll and resumeAll a thing? That would be pretty useful when I pause my game, which is an artificial pause and not FlxG.pause() or however you do it.

@IBwWG
Copy link
Contributor Author

IBwWG commented Mar 18, 2016

Well, if they are, they aren't a thing in this PR... :) I haven't gotten around to pausing implementations in my games yet...can I ask why you do an artificial pause and not the official one? (I have a feeling the official one will be better integrated overall than making a new method, but I'm curious...and maybe there's a way to customize pausing that still benefits from the official implementation too.)

@Gama11
Copy link
Member

Gama11 commented Mar 18, 2016

I'm not sure if this makes sense for looped tweens / timers, they can't really be "completed".

@IBwWG
Copy link
Contributor Author

IBwWG commented Mar 18, 2016

Good point. I can think of a scenario where you'd have both kinds going and would want to only touch the non-loopers.

{
for (timer in _timers)
timer.onComplete(timer);
Copy link
Member

Choose a reason for hiding this comment

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

This completely bypasses all the timer logic. The user might check properties of the timer on the callback (one of the reasons why it's passed as an argument), and they would be inconsistent with how they would look like on normal completion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know. I think the method I used for tweens could work here, eh?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure, looks similarly hacky.

Copy link
Member

Choose a reason for hiding this comment

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

(definitely better though)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you be more specific? I thought they were dissimilar, otherwise my question of whether to use the other method made no sense... IMO they are dissimilar in that the tweens are update()d to the end, which updates all the properties properly (which addressed your concern about bypassing logic, I thought), unlike here where I just called the callback without bothering with any properties.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's a better approach.. a bit unusual, but probably safe. And it might really just feel unusual because it wasn't possible to do this sort of thing before too long ago (you didn't have per-object control of elapsed until that argument was added to update()).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, fair enough.

Unfortunately, I have trouble getting it to work the other way:

    public function completeAll():Void
    {
        for (timer in _timers)
            if (timer.loops == 1)
                timer.update(timer.timeLeft);
    }

...this somehow only fires about half my timers, whereas the original approach was working fine for me, somehow.

Really bizarre stuff, because I add them all at once:

        for (i in 0...randomizedLetters.length)
            new FlxTimer().start(0.5 + i * 0.05, function(?ignored:FlxTimer) { ...

But then if I add trace statements:

    public function completeAll():Void
    {
        trace("start");
        for (timer in _timers){
            trace("loops", timer.loops, "time", timer.time);
            if (timer.loops == 1)
                timer.update(timer.timeLeft);
        }
    }

I get this output, then, having to press a key twice to activate it, which it does in two batches...and still doesn't complete all the timers in the end! :

FlxTimer.hx:282: start
FlxTimer.hx:284: loops,1,time,3.45
FlxTimer.hx:284: loops,1,time,3.4000000000000004
FlxTimer.hx:284: loops,1,time,3.35
FlxTimer.hx:284: loops,1,time,3.3000000000000003
FlxTimer.hx:284: loops,1,time,3.25
FlxTimer.hx:284: loops,1,time,3.2
FlxTimer.hx:284: loops,1,time,0.8
FlxTimer.hx:284: loops,1,time,0.8500000000000001
FlxTimer.hx:284: loops,1,time,0.9
FlxTimer.hx:284: loops,1,time,0.95
FlxTimer.hx:284: loops,1,time,1
FlxTimer.hx:284: loops,1,time,1.05
FlxTimer.hx:284: loops,1,time,1.1
FlxTimer.hx:284: loops,1,time,1.15
FlxTimer.hx:284: loops,1,time,1.2000000000000002
FlxTimer.hx:284: loops,1,time,1.25
FlxTimer.hx:284: loops,1,time,1.3
FlxTimer.hx:284: loops,1,time,1.35
FlxTimer.hx:284: loops,1,time,1.4
FlxTimer.hx:284: loops,1,time,1.4500000000000002
FlxTimer.hx:284: loops,1,time,1.5
FlxTimer.hx:284: loops,1,time,1.55
FlxTimer.hx:284: loops,1,time,1.6
FlxTimer.hx:284: loops,1,time,1.6500000000000001
FlxTimer.hx:284: loops,1,time,1.7000000000000002
FlxTimer.hx:284: loops,1,time,1.75
FlxTimer.hx:284: loops,1,time,1.8
FlxTimer.hx:284: loops,1,time,1.85
FlxTimer.hx:282: start
FlxTimer.hx:284: loops,1,time,3.1500000000000004
FlxTimer.hx:284: loops,1,time,3.1
FlxTimer.hx:284: loops,1,time,3.0500000000000003
FlxTimer.hx:284: loops,1,time,3
FlxTimer.hx:284: loops,1,time,2.95
FlxTimer.hx:284: loops,1,time,2.9000000000000004
FlxTimer.hx:284: loops,1,time,2.85
FlxTimer.hx:284: loops,1,time,2.8000000000000003
FlxTimer.hx:284: loops,1,time,2.75
FlxTimer.hx:284: loops,1,time,2.7
FlxTimer.hx:284: loops,1,time,2.65
FlxTimer.hx:284: loops,1,time,2.6
FlxTimer.hx:284: loops,1,time,2.5500000000000003

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I don't know what could be causing this. To me, it's still a very useful function as I have it currently in the PR, and it therefore could probably be of use to someone else (after all, there's that undocumented code in there to allow negative numbers of loops...surely that originated the same way ;) ) I propose keeping it, with a caveat, which I've just added, until such time as someone has time to widen its applicability.

Copy link
Member

Choose a reason for hiding this comment

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

You can't iterate on timers while also removing them from the timer array (which update()->cancel() does), if this is the issue here. It's currently an issue in github, but you would have to manually onComplete all the timers and then cancel all the timers in another loop.

It's like when you kill an enemy; you loop through all the enemies, push the dead ones into a temp array, and then loop through the temp array and remove those from the main one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! That makes sense. Thanks for explaining. Maybe a while loop could work here. Unless we want only non-loopers like in the tweens. Then a temp array is the only way to go. Will do.

@IBwWG
Copy link
Contributor Author

IBwWG commented Mar 18, 2016

OK, this seemed to work well in my project, at least, and should correctly provide endpoint time properties to callback functions.

GimmickyApps added 2 commits March 19, 2016 17:46
Useful for other contexts than completing.
@IBwWG
Copy link
Contributor Author

IBwWG commented Mar 19, 2016

I threw in a couple more, so users can get at the managers' members for other purposes than skipping to the end.

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 19, 2016

Could someone help me understand what's actually failing on the build? It says zero failures and errors, and then looks as if it was aborted manually just after finishing, or something.

@Gama11
Copy link
Member

Gama11 commented Apr 19, 2016

https://travis-ci.org/HaxeFlixel/flixel/jobs/122162083#L1748-L1750

No idea what that's about.. seems to happen randomnly every now and then.

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 19, 2016

Thanks yeah I should've quoted the lines.

pure virtual method called

terminate called without an active exception

Aborted (core dumped)

OK, well, I'll try a different equivalent way, whether it's actually a fix or just triggering a rebuild would have done it no matter what, well, who knows (assuming it works this time...)

@Gama11
Copy link
Member

Gama11 commented Apr 19, 2016

That failure has almost certainly nothing to do with your changes.

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 20, 2016

OK, well, anyway, it's passing.

Catch up with upstream.
FlxTilemapTest: add a basic test for loadMapFromGraphic()
Catch up with upstream.
Fix VCR to record simultaneous keys+mouse, closes HaxeFlixel#1739 (HaxeFlixel#1825)
Catch up with upstream.
@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 23, 2016

@MSGhero it's not in this PR, but I have ended up adding a pause screen artificially too, but more so because I completely forgot to look whether there was already a built-in way of doing this, because I was reusing code from an in-game editor I had whipped up.

Anyway, if you use this PR, it includes generic forEach functions, which let you do what I ended up doing for my pause screen:

    public function togglePause() {
        forEach(function(s:FlxBasic) {
            s.active = !s.active;
        });
        FlxTween.manager.forEach(function(s:FlxTween) {
            s.active = !s.active;
        });
        FlxTimer.manager.forEach(function(s:FlxTimer) {
            s.active = !s.active;
        });
        FlxG.sound.list.forEach(function(snd){
            if (snd.playing)
                snd.pause();
            else
                snd.resume();
        });
    }

The advantage of the forEach methods is that you don't have to build an array of the things you want paused, since the Flx*Managers are already doing that for you anyway. This just gives you handy access to it in the same way that already exists for other types, like your state's members, and FlxG.sound.list as you can see here (and already use without this PR.)

@Gama11
Copy link
Member

Gama11 commented Apr 23, 2016

Why not set active of the managers themselves, instead of iterating over all members?

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 23, 2016

I hadn't noticed you could do that. Thanks! I guess then:

    public function togglePause() {
        forEach(function(s:FlxBasic) {
            s.active = !s.active;
        });
        FlxTween.manager.active = !FlxTween.manager.active;
        FlxTimer.manager.active = !FlxTimer.manager.active;
        FlxG.sound.list.forEach(function(snd){
            if (snd.playing)
                snd.pause();
            else
                snd.resume();
        });
    }

...which you can do without this PR. :)

Although, the forEach functions are still generally useful for other cases. :)

@MSGhero
Copy link
Member

MSGhero commented Apr 23, 2016

Maybe you don't want them all disabled. Or maybe your pause screen requires tweens.

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 23, 2016

There you go, valid point. Customize as needed. :)

@MSGhero
Copy link
Member

MSGhero commented Apr 23, 2016

But yeah, forEach 👍

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 23, 2016

Quite true what you just said, I put that code in there without thinking, and forgot I started a tween just after that. Of course, the tween didn't work, because forEach doesn't do quite the same thing...so thanks for pointing that out :)

Catch up with upstream.
@IBwWG IBwWG changed the title "Complete all timers" helper function "Complete all timers" and "complete all tweens" helper functions Jun 20, 2016
@IBwWG
Copy link
Contributor Author

IBwWG commented Aug 10, 2016

I don't think this PR breaks anything, it only adds functionality a la similar areas in the rest of flixel, no?

@Gama11 Gama11 merged commit dfc8470 into HaxeFlixel:dev Aug 21, 2016
*
* Note: if they haven't yet begun, this will first trigger their onStart callback.
*
* Note: their onComplete callbacks are triggered in the next frame. To trigger them immediately, call `FlxTween.manager.update(0);` after this function.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the update(0) hint, seems like a hack.

Copy link
Member

Choose a reason for hiding this comment

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

Excellent docs otherwise. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you have it so we call update(0) in the library, instead? It's been a while so I forget the reason (if there was one) for adding a hint rather than just doing it in the library, which would have been easier...I'm trying to think if there's some case you'd want to get it to the end but have the callback occur on the next frame. I know that for at least one usage I needed it to occur this frame, because after the onComplete possible set up new tweens and timers, I needed to have them skip to their endings, too, in a cascade. Maybe I just didn't want to assume this was needed, but couldn't think of a use case then, either...?

Copy link
Member

Choose a reason for hiding this comment

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

It does seem like a weird gotcha you have to be aware of. What's the reason it's delayed in the first place anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's just how tweens work, c.f. the the rest of the file, I can't remember exactly anymore

@Gama11
Copy link
Member

Gama11 commented Aug 21, 2016

I removed FlxTimerManager's completeAll() again for now in 1980cd8, it doesn't work as I'd expect it to (I think it should complete all timers, expect ones that are looping indefinitely). Can you create a new pull request with that functionality? Unit tests would be a nice bonus.

The forEach() functions are fairly non-problematic, but so would writing tests for them be. ;)

FlxTweenManager's completeAll() will fail if the tween type is used as a bit field, for instance a backwards oneshot tween: FlxTween.ONESHOT | FlxTween.BACKWARD. I'm gonna fix that in the next commit.

Gama11 added a commit that referenced this pull request Aug 21, 2016
It wouldn't have completed a FlxTween.ONESHOT | FlxTween.BACKWARD tween before.
see #1782
@IBwWG IBwWG deleted the complete-all-timers branch August 22, 2016 06:22
@IBwWG
Copy link
Contributor Author

IBwWG commented Aug 22, 2016

In the case of completing finite looping timers, how many times would you run their onComplete--once, or once per remaining loop?

@Gama11
Copy link
Member

Gama11 commented Aug 22, 2016

Good question. I suppose it makes sense to run it as often as it would have in normal execution?

@IBwWG
Copy link
Contributor Author

IBwWG commented Aug 22, 2016

OK, sounds good.

@IBwWG
Copy link
Contributor Author

IBwWG commented Sep 7, 2016

Done: #1933

Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
It wouldn't have completed a FlxTween.ONESHOT | FlxTween.BACKWARD tween before.
see HaxeFlixel#1782
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.

3 participants