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

Phaser.Math.Wrap inconsitent behaviour #6479

Closed
EmilSV opened this issue Apr 19, 2023 · 4 comments
Closed

Phaser.Math.Wrap inconsitent behaviour #6479

EmilSV opened this issue Apr 19, 2023 · 4 comments

Comments

@EmilSV
Copy link
Contributor

EmilSV commented Apr 19, 2023

Version

  • Phaser Version: 3.60

Description

After the 3.60 wrap change to have check that made sure if it was in the range it return the same result.

    if (value >= min && value <= max)
    {
        //  Skip modulo if already in range
        return value;
    }

But if that will lead to inconsistent behavior if the value is out of range for example
wrap(4,0,4) == 4
wrap(8,0,4) == 0
But is should be the same because 8 is double of 4.

Before 3.60 the check was not there and wrap was consistent.

Example Test Code

console.log(Phaser.Math.Wrap(4,0,4)); // 4
console.log(Phaser.Math.Wrap(8,0,4)); // 0
console.log(Phaser.Math.Wrap(8,0,4) === Phaser.Math.Wrap(4,0,4)); //false

Additional Information

I don't know if this it intended behavior or a bug, if it is bug the i gladly make a pull request with the required changes

@Gathaa
Copy link

Gathaa commented Apr 23, 2023

You could modify the code to wrap values that are greater than the maximum value back to the minimum value, and values that are less than the minimum value back to the maximum value. when the range is 0 you return the minimum value basically the range is max - min and then you loop over a result to the function of the range if you receive the result less than the minimum you increment it to the function of the range else you decrement it to the function of the range . if you seem confused with the answer i will be pleased to answer your questions .

@vforsh
Copy link
Contributor

vforsh commented Apr 30, 2023

I agree with @EmilSV, seems like a bug to me.

Previous Wrap version was perfect for wrapping array indices. New version doesn't do it and I always thought that it was the main use case for Wrap.

// array length = 10 (so last index is 9)

// Wrap before 3.60
Wrap(-1, 0, 10) // 9
Wrap(10, 0, 10) // 0

// new Wrap
Wrap(-1, 0, 10) // 9
Wrap(10, 0, 10) // 10

@wayfu
Copy link

wayfu commented Jul 16, 2023

This example also shows the behavior: https://labs.phaser.io/view.html?src=src/display/blend%20modes/blend%20mode%20tester.js&v=3.60.0-beta.19

You can only reach 0 when counting down, when counting up it goes from 14 to 15 ("undefined"), and then directly to 1, skipping 0. The correct behavior happens when setting the min/max to -1 and 14.

@photonstorm
Copy link
Collaborator

Thank you for submitting this issue. We have fixed this and the fix has been pushed to the master branch. It will be part of the next release. If you get time to build and test it for yourself we would appreciate that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants