Skip to content

FlxTween framerate option #3372

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

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

FlxTween framerate option #3372

wants to merge 1 commit into from

Conversation

cyn0x8
Copy link
Contributor

@cyn0x8 cyn0x8 commented Feb 20, 2025

implements / closes #3367

@Raltyro
Copy link

Raltyro commented Feb 20, 2025

I think you could change the capitilzation to "frameRate" in favor of FlxAnimation

@Geokureli
Copy link
Member

I think you could change the capitilzation to "frameRate" in favor of FlxAnimation

good to be consistent, but it seems flixel is already inconsistent on this
https://github.com/HaxeFlixel/flixel/blob/dev/flixel/FlxG.hx#L127-L134

https://github.com/search?q=repo%3AHaxeFlixel%2Fflixel%20framerate&type=code
flash/openfl use capital R, I personally like lowercase, when we come to a decision we should rename the existing ones in a different PR

@Raltyro
Copy link

Raltyro commented Feb 20, 2025

fair enough, that works too

@@ -505,6 +505,7 @@ class FlxTween implements IFlxDestroyable
public var active(default, set):Bool = false;
public var duration:Float = 0;
public var ease:EaseFunction;
public var framerate:Null<Float>;
Copy link
Member

Choose a reason for hiding this comment

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

Let's use a float where 0 means "an infinite rate "

Suggested change
public var framerate:Null<Float>;
public var framerate:Float;

@@ -562,6 +563,7 @@ class FlxTween implements IFlxDestroyable
onUpdate = Options.onUpdate;
onComplete = Options.onComplete;
ease = Options.ease;
framerate = Options.framerate;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
framerate = Options.framerate;
framerate = Options.framerate != null ? Options.framerate : 0;

var delay:Float = (executions > 0) ? loopDelay : startDelay;
if (_secondsSinceStart < delay)
{
return;
}
scale = Math.max((_secondsSinceStart - delay), 0) / duration;

if (framerate != null && framerate > 0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (framerate != null && framerate > 0)
if (framerate > 0)

* This also affects how often `onUpdate` is called.
* Not to be confused with `period` for flickering.
*/
@:optional var framerate:Float;
Copy link
Member

Choose a reason for hiding this comment

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

nit picky, but I'm trying to use Null whenever it's implied

Suggested change
@:optional var framerate:Float;
@:optional var framerate:Null<Float>;

Comment on lines +624 to +625
var preTick:Float = _secondsSinceStart;
var postTick:Float = (_secondsSinceStart += elapsed);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var preTick:Float = _secondsSinceStart;
var postTick:Float = (_secondsSinceStart += elapsed);
var preTick:Float = _secondsSinceStart;
_secondsSinceStart += elapsed
var postTick:Float = _secondsSinceStart;

* Optional set framerate for this tween to update at.
* This also affects how often `onUpdate` is called.
*/
@:optional var framerate:Float;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@:optional var framerate:Float;
@:optional var framerate:Null<Float>;

Comment on lines +439 to +453
var tweenUpdatesDefault:Int = 0;
var tweenDefault:FlxTween = FlxTween.tween(this, {value: 1000}, 2, {startDelay: 0.54321, onUpdate: function(_) tweenUpdatesDefault++});
step(FlxG.updateFramerate * 3); // 3 full seconds
Assert.areEqual(FlxG.updateFramerate * 2, tweenUpdatesDefault);

// various tween framerate values
var testFramerates:Array<Float> = [5, 10, 10.1, 24, 29.9, 30, 30.1, 31, 59.9, 60, 100];
for (framerate in testFramerates)
{
var tweenUpdates:Int = 0;
var tween:FlxTween = FlxTween.tween(this, {value: 1000}, 2, {startDelay: 0.54321, framerate: framerate, onUpdate: function(_) tweenUpdates++});
step(FlxG.updateFramerate * 3); // 3 full seconds
Assert.areEqual(Std.int(Math.ceil(Math.min(framerate, FlxG.updateFramerate) * 2)), tweenUpdates);
}
}
Copy link
Member

@Geokureli Geokureli Feb 21, 2025

Choose a reason for hiding this comment

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

it's a little difficult to understand what this is testing, and generally, we should assert against literal values rather than an equation. Also try to avoid "throwing the kitchen sink" at every test, make it very obvious what each test covers and why it's needed

Example (untested):

function assertTweenUpdates(duration:Float, ?fps:Float, ?options:TweenOptions, expected:Int)
{
	var updates = 0;
	
	if (options)
		options = {};
	
	if (fps != null)
		options. framerate = fps;
	
	options.onUpdate = function(_) updates++;
	options.onComplete = function(_) Assert.areEqual(expected, updates);
	FlxTween.tween({value: 0.0}, {value: 1}, 2.0, options);
}

assertTweenUpdates(2.0, null, null, 60);
assertTweenUpdates(2.0, 0, null, 60);
assertTweenUpdates(2.0, 5, null, 10);
assertTweenUpdates(2.0, 10, { startDelay: 0.5 }, 20);
assertTweenUpdates(2.0, 10.1, null, 21);
assertTweenUpdates(2.0, 29.9, null, 60);
assertTweenUpdates(2.0, 100, null, 60);
//... other edge cases

step(FlxG.updateFramerate * 3); // 3 full seconds

@cyn0x8 cyn0x8 marked this pull request as draft February 21, 2025 10:36
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.

FlxTween framerate feature
3 participants