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

Simplify and cleanup in small areas #2755

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Conversation

CrazyInfin8
Copy link
Contributor

This isn't the most important PR since it doesn't really fix anything or improve performance by much, but I found a few cases where code could be simplified a tiny bit.

Each commit explains what changed and why but essentially:

  1. if (!Force && !finished && reversed == Reversed)
    {
        paused = false;
        finished = false; // finished is already false due to if condition
        return;
    }
  2. function get_hue():Float
    {
        var hueRad = Math.atan2(Math.sqrt(3) * (greenFloat - blueFloat), 2 * redFloat - greenFloat - blueFloat);
        var hue:Float = 0;
        if (hueRad != 0)
        {
            // The Math.atan2 part of below is already calculated by hueRad
            hue = 180 / Math.PI * Math.atan2(Math.sqrt(3) * (greenFloat - blueFloat), 2 * redFloat - greenFloat - blueFloat);
            hue = 180 / Math.PI * hueRad;
        }
    
        return hue < 0 ? hue + 360 : hue;
    }
  3. in FlxColor.setHSChromaMatch(), the saturation field isn't used and could be removed. (I decided to rename it setHueChromaMatch() but the name doesn't really matter)

`finished` must be false in order for this to run which sets it to false again.
`hueRad` calculated this value already so, for readability, we don't need to put the whole formula again.
@CrazyInfin8
Copy link
Contributor Author

Is it possible to rerun those failing checks? it looks as if they failed because Haxelib couldn't download the necessary packages at the time that the tests were running.

@Geokureli Geokureli merged commit 38d1144 into HaxeFlixel:dev Apr 5, 2023
@Geokureli
Copy link
Member

Thanks!

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