Incorrect modulo behavior in plain Jass

[nvs]

The 1.29 patch introduced modulo (%) to Jass. Will this functionality be more thoroughly supported by Wurst in the future?

// Jass
local real r = 5 % 3 // No error. But converted to `ModuloReal`.
local integer i = 5 % 3 // Error, although valid.

Wrong argument for parameter i: expected int, but found real.

Also, the results do differ.

call BJDebugMsg(I2S( 5 %  3)) //  2
call BJDebugMsg(I2S(-5 %  3)) // -2
call BJDebugMsg(I2S( 5 % -3)) //  2
call BJDebugMsg(I2S(-5 % -3)) // -2

call BJDebugMsg(I2S(ModuloInteger( 5,  3))) //  2
call BJDebugMsg(I2S(ModuloInteger(-5,  3))) //  1
call BJDebugMsg(I2S(ModuloInteger( 5, -3))) //  2
call BJDebugMsg(I2S(ModuloInteger(-5, -3))) // -5

[Frotty]

The 1.29 patch introduced modulo (%) to Jass. Will this functionality be more thoroughly supported by Wurst in the future?

Modulo has been supported in wurst for far longer already. To keep backwards compatibility and prevent braking changes we will continue to translate it into the function instead of using the new operator.

5 % 3 is not valid. Just like / and div modulo is separated for int and real in wurst, % and mod.

For Jass compat it should probably be adjusted. For Wurst I don't think it brings any benefit.

[nvs]

5 % 3 is not valid. Just like / and div modulo is separated for int and real in wurst, % and mod.

My example was with respect to Jass, and Jass only. I edited my example with that in mind.

[peq]

We can fix this for Jass code. Is the behavior of x % y and ModuloReal(x,y) equivalent?

[nvs]

Well, I had not tried the operator for reals yet. I just assumed it would also work. Now, I normally don't do things in the World Editor at all, otherwise I'd have seen this sooner. Trying to pass a real to % in Jass throws up the following error upon save:

Invalid type for the specified operator.

// Jass
local real a = 2
local real b = 2
call BJDebugMsg(R2S(a % b)) // Error.
call BJDebugMsg(R2S(6.0 % 2.0)) // Error

Trying to feed the game a map with such code doesn't work either (which is what I was running into before I thought to open the WE). It works fine with integers, though. So, current Jass behavior supports the modulo operator for integers but not reals. And Wurst reads Jass code that uses it for reals but not integers.

Curiously, Wurst converts the following (for reals) in a manner that is consistent with the truncation performed by the % operator on integers in Jass (as seen in my earlier examples).

// Jass
local real a = 5.0
local real b = 3.0

// Jass input. But errors as is in the WE.
call BJDebugMsg(R2S( a %  b))
call BJDebugMsg(R2S(-a %  b))
call BJDebugMsg(R2S( a % -b))
call BJDebugMsg(R2S(-a % -b))

// Wurst output.
call BJDebugMsg(R2S(ModuloReal(a, b))) // 2.000
call BJDebugMsg(R2S( - ModuloReal(a, b))) // -2.000
call BJDebugMsg(R2S(ModuloReal(a,  - b))) // 2.000
call BJDebugMsg(R2S( - ModuloReal(a,  - b))) // -2.000

And to use one of the examples for ModuloReal() in the blizzard.j to highlight Wurst's existing behavior:

// Jass
local real a = 6.0
local real b = 2.5

// These get converted to `ModuloReal()` by Wurst.
call BJDebugMsg(R2S( a %  b)) // 1.000
call BJDebugMsg(R2S(-a %  b)) // -1.000
call BJDebugMsg(R2S( a % -b)) // 1.000
call BJDebugMsg(R2S(-a % -b)) // -1.000

call BJDebugMsg (R2S (ModuloReal ( a,  b))) // 1.000
call BJDebugMsg (R2S (ModuloReal (-a,  b))) // 1.500
call BJDebugMsg (R2S (ModuloReal ( a, -b))) // 1.000
call BJDebugMsg (R2S (ModuloReal (-a, -b))) // -3.500

Knowing this, I can't say what is the correct course is at this point. I don't believe the modulo operator was announced officially in 1.29. Someone stumbled upon it, if I recall. Like I said, I just assumed it worked for both integers and reals.

[Frotty]

The modulo added to jass is just for ints. I'm not sure if it wasn't mentioned in changelogs, but it's quite common knowledge and @lep added it to pjass some time ago. We should probably just disable wurst resolving modulo in jass files. The only issue is that it would be a breaking change now for reals.