Implement FlxSprite.skew

[DetectiveBaldi]

Closes: #3238

Implementation of the skew property from flixel.addons.effects.FlxSkewedSprite.

This implementation is missing matrixExposed and transformationMatrix. I can't really see a use case for these variables, nor have I seen them used in any project.

If this is merged:

• We need to deprecate FlxSkewedSprite, and remove some conflicting fields.
• We need to either remove the FlxSkewedSprite demo, or adjust it to use FlxSprite.skew.

[DetectiveBaldi]

test project: SkewedSpriteTest.zip
(left/right to change skew, x and y values displayed in debugger)

[Geokureli]

I wonder if we should just add a matrix field rather than just skew, and then add a skew method to FlxMatrix

[DetectiveBaldi]

I wonder if we should just add a matrix field rather than just skew, and then add a skew method to FlxMatrix

This seems like the ideal solution

[DetectiveBaldi]

figured it out

edit:

I wonder if we should just add a matrix field rather than just skew, and then add a skew method to FlxMatrix

it looks like the matrix skewing have to be called every time drawComplex is, i'm not sure how i would work around that without an extra field for skew

[Geokureli]

Here's an example of what I'm suggesting. they are implemented in extending classes, but I'm imagining adding these fields to FlxMatrix and FlxSprite

package states;

import flixel.FlxCamera;
import flixel.FlxG;
import flixel.math.FlxAngle;
import flixel.math.FlxMatrix;
import flixel.text.FlxText;

class SpriteMatrixTestState extends flixel.FlxState
{
    final sprite = new MatrixSprite("assets/images/haxe.png");
    final skewedSprite = new flixel.addons.effects.FlxSkewedSprite("assets/images/haxe.png");
    var scaleX = 1.0;
    var scaleY = 1.0;
    var skewX = 0.0;
    var skewY = 0.0;
    var tX = 0.0;
    var tY = 0.0;
    
    public function new()
    {
        super();
        
        sprite.screenCenter();
        sprite.x -= sprite.width;
        add(sprite);
        
        skewedSprite.screenCenter();
        skewedSprite.x += sprite.width;
        add(skewedSprite);
        
        final text = new FlxText("MatrixSprite", 16);
        text.x = sprite.x;
        text.y = sprite.y + sprite.height * 1.5;
        add(text);
        
        final text = new FlxText("FlxSkewedSprite", 16);
        text.x = skewedSprite.x;
        text.y = skewedSprite.y + skewedSprite.height * 1.5;
        add(text);
        
        FlxG.watch.addFunction("scale", ()->'( x: $scaleX | y: $scaleY )');
        FlxG.watch.addFunction("skew", ()->'( x: $skewX | y: $skewY )');
        FlxG.watch.addFunction("t", ()->'( x: $tX | y: $tY )');
        
        FlxG.log.notice
            ( "ARROWS: Translate\n"
            + "ARROWS + SHIFT: Scale\n"
            + "ARROWS + ALT: Skew"
            );
        FlxG.debugger.visible = true;
    }
    
    override function update(elapsed:Float)
    {
        super.update(elapsed);
        
        final L = FlxG.keys.pressed.LEFT;
        final R = FlxG.keys.pressed.RIGHT;
        final U = FlxG.keys.pressed.UP;
        final D = FlxG.keys.pressed.DOWN;
        
        if (FlxG.keys.pressed.SHIFT)
        {
            if (L) scaleX -= 0.01;
            if (R) scaleX += 0.01;
            if (U) scaleY += 0.01;
            if (D) scaleY -= 0.01;
        }
        else if (FlxG.keys.pressed.ALT)
        {
            if (L) skewX -= 0.01;
            if (R) skewX += 0.01;
            if (U) skewY -= 0.01;
            if (D) skewY += 0.01;
        }
        else
        {
            if (L) tX -= 1;
            if (R) tX += 1;
            if (U) tY -= 1;
            if (D) tY += 1;
        }
        
        sprite.transform.identity();
        sprite.transform.skewRadians(skewX * Math.PI / 2, skewY * Math.PI / 2);
        sprite.transform.scale(scaleX, scaleY);
        sprite.transform.translate(tX, tY);
        
        skewedSprite.skew.set(skewX * 90, skewY * 90);
    }
}

class SkewMatrix extends FlxMatrix
{
	public inline function isIdentity()
	{
		return equals(openfl.geom.Matrix.__identity);
	}
	
	public function skewRadians(skewX:Float, skewY:Float)
	{
		b = Math.tan(skewY);
		c = Math.tan(skewX);
	}
	
	public inline function skewDegrees(skewX:Float, skewY:Float)
	{
		skewRadians(skewY * FlxAngle.TO_RAD, skewX * FlxAngle.TO_RAD);
	}
}

class MatrixSprite extends flixel.FlxSprite
{
    public var transform:SkewMatrix = new SkewMatrix();
    
    override function isSimpleRenderBlit(?camera:FlxCamera):Bool
    {
        return transform.isIdentity() || super.isSimpleRenderBlit(camera);
    }
    
    override function drawComplex(camera:FlxCamera)
    {
        _frame.prepareMatrix(_matrix, ANGLE_0, checkFlipX(), checkFlipY());
        _matrix.translate(-origin.x, -origin.y);
        _matrix.scale(scale.x, scale.y);
        
        if (bakedRotationAngle <= 0)
        {
            updateTrig();
        
            if (angle != 0)
                _matrix.rotateWithTrig(_cosAngle, _sinAngle);
        }
        
        _matrix.concat(transform);
        getScreenPosition(_point, camera).subtractPoint(offset);
        _point.add(origin.x, origin.y);
        _matrix.translate(_point.x, _point.y);
        
        if (isPixelPerfectRender(camera))
        {
            _matrix.tx = Math.floor(_matrix.tx);
            _matrix.ty = Math.floor(_matrix.ty);
        }
        
        camera.drawPixels(_frame, framePixels, _matrix, colorTransform, blend, antialiasing, shader);
    }
}

[DetectiveBaldi]

👍

[DetectiveBaldi]

CI is still failing, on flash it can't find equals and __identity, not sure about other targets
edit: looks like other targets are failing due to interface issues with flixel-ui

[Geokureli]

Also, one thing I forgot to ask, why is it that you want this new feature in FlxSprite, instead of just using FlxSkewedSprite?

[DetectiveBaldi]

Also, one thing I forgot to ask, why is it that you want this new feature in FlxSprite, instead of just using FlxSkewedSprite?

FlxSkewedSprite was the only class i used from flixel-addons, and it felt weird to ship a project of mine with an extra dependency for one class
there's also the issue of text skewing not being possible without extension, and i think skewing (or at this point a variable for simple rendering transformations) is commonplace enough to warrant an addition to FlxSprite

[Geokureli]

Also, one thing I forgot to ask, why is it that you want this new feature in FlxSprite, instead of just using FlxSkewedSprite?

FlxSkewedSprite was the only class i used from flixel-addons, and it felt weird to ship a project of mine with an extra dependency for one class

1. There's not really a downside to including flixel-addons for a single class, the unused parts don't affect compilation or the final build
2. You can avoid the import by using the classes I provided above, and I assume you don't need to worry about flash in your project

there's also the issue of text skewing not being possible without extension

Is this something you've needed?

and i think skewing is commonplace enough to warrant an addition to FlxSprite

Based on what? Anecdotally, I really never see people requesting this.

The reason I'm hesitant about this is that FlxSprite is already pretty bloated, I wanna transition to a more compositional system for flixel, rather than adding every feature that someone might need into FlxBasic, FlxObject and FlxSprite. Right now it's pretty easy to add this feature to a FlxSprite extending class, rather than adding extra memory to every FlxSprite for something that: A. Doesn't seem widely used and B. Can be implemented in a few different ways (We've discussed 2 valid solutions but I've considered others, namely a third that I would like in my projects, but I don't think others would like).

In the end, we need to consider what problem a change like this fixes, and the only problems I see are:

1. It feels weird to use flixel-addons for 1 feature
2. I would need to implement it myself if I don't want to use FlxSkewedSprite

1 seems like a non-issue and 2 is trivial, especially since there's already an example implementation to copy.

Thoughts?

[DetectiveBaldi]

there's also the issue of text skewing not being possible without extension

Is this something you've needed?

yes

if it is too much trouble to merge here, i can result to the extra extension
personally, i don't believe flxsprite is too bloated, for the most part every feature there serves an important purpose
what examples do you have in mind?

[Geokureli]

what examples do you have in mind?

you mean the "example implementation to copy" i mentioned? that example is FlxSkewedSprite

if it is too much trouble to merge here, i can result to the extra extension personally, i don't believe flxsprite is too bloated, for the most part every feature there serves an important purpose

"bloat" doesn't mean "features that serve no purpose", it can mean a lot of ad-hoc solutions that most people don't need, making it harder to implement your own features or different implementations of existing features. This is why moving to composition style system can be really beneficial, such a system would allow you to make a skew component that can be attached to either a FlxSprite or a FlxText or what have you

[DetectiveBaldi]

what examples do you have in mind?

you mean the "example implementation to copy" i mentioned? that example is FlxSkewedSprite

i meant what features in FlxSprite make it difficult to add new functionality (and where you might need to add newadjust said functionality)
i remember some discussion of removing health related fields from FlxBasic for the sake of ease of customization, and i think that's a fair example, but what cases of this are present in FlxSprite?

im not too familiar with the concept of composition based programming, ill look into it more

(Edit from GK, i accidentally edited your post instead of replying to it, hopefully I've reverted that edit)

[Geokureli]

Similarly to how having a health field in FlxObject made it harder to implement a different health system in your game (simply by taking the field names health and hurt) a built in skew/transform system makes it harder to make a custom system using those field names