jassdoc icon indicating copy to clipboard operation
jassdoc copied to clipboard

`GetRandomInt` not swapping argument values

Open WaterKnight opened this issue 1 year ago • 4 comments

There is this note on GetRandomInt:

@note If lowBound > highBound then it just swaps the values. https://github.com/lep/jassdoc/blob/e0ae9a48607b54290f8c57f1d8958a1ca69fad7e/common.j#L17280C1-L17280C61

I could not confirm this in a recent version (1.36.1.21015) as pointed out here: https://github.com/lep/jassdoc/issues/151#issuecomment-2143869913

Rather, it seems similar to how GetRandomReal behaves.

WaterKnight avatar Jun 20 '24 20:06 WaterKnight

Indeed. Added in 38ae32affacb91bbb0e553fe90c2f4b5ab3e34a0. Maybe it woul be worthwhile to point out the patch this change happened because i'm quite certain it didn't happen in older patches. Also to me i would think that GetRandomInt is the more often use function so it might be better to have the "main" documentation on GetRandomInt instead of GetRandomReal but it's fine either way.

lep avatar Jun 22 '24 09:06 lep

I don't have access to my game version library at the moment. Maybe the behavior didn't change and the initial note was wrong? Please keep this open and I will check it some time down the line.

Luashine avatar Jun 22 '24 10:06 Luashine

scope test initializer init
private function onTimer takes nothing returns nothing
    call SetRandomSeed(0)
    call BJDebugMsg(I2S(GetRandomInt(-10, 10)))
    call SetRandomSeed(0)
    call BJDebugMsg(I2S(GetRandomInt(10, -10)))
endfunction
private function init takes nothing returns nothing
    call TimerStart(CreateTimer(), 1, false, function onTimer)
endfunction
endscope

print image at war3 version

1.24.4.6387
1.26.0.6401
1.27.0.52240
1.28.5.7680
1.29.2.9208 Public Test
1.29.2.9231
1.30.1.10211
1.31.1.12164
1.32.10.18820
1.36.1.21015 SD
1.36.2.21230

shuen4 avatar Jun 22 '24 14:06 shuen4

Yes the GetRandomReal's min/mid/max formula is mostly correct. For the -Int mid the game's calculation is the same as // 2 (integer division in Lua) or math.floor(mid) -- rounding towards -inf. Keep in mind the integer syntax is new to Lua, I left a comment about it below.

Except for the 0.5f error for mid values, it's correct to link to GetRandomReal.

There's another problem - and I didn't test the same for floats - if the minimum value is too low, then it will have an effect on the possible output values:

print(GetRandomInt(math.mininteger, math.maxinteger))
--> ALWAYS math.mininteger

See "Max output is offset when using extreme values" below. Personally I don't want to test these any more :/ nor any other extremes. So what can be added right now:

  • new seeds for the GetRandomInt
  • corrected min/mid/max formula
  • note about extreme values, linking to this issue

PS: @shuen4 you tested too many versions 😅 it's safe to assume if it didn't change between 1.24 and 1.36 then it was not changed in between. The only other big changes could have been expected with TFT, but I doubt they changed many internals there... with the exception of simply adding more systems to the game. Rather you should assume the jassdoc descriptions to be incomplete ;)

Test formula for the 3 found seeds

function testFormulaInt()
	local testRInt = function (seed, low, high)
		SetRandomSeed(seed);
		return string.format("%.16f", GetRandomInt(low,high))
	end
	
	local reFormula = function(low, high)
		local min = low
		local mid
		local max = (low > high) and (2 * low - high) or high
		if low == high then
			mid = low
		elseif low < high then
			-- you can use math.floor rather than integer division
			mid = (low + high) // 2
		else -- low > high
			-- you can use math.floor rather than integer division
			mid = (low + -high) // 2 + low
		end
		
		return min, mid, max
	end
	
	local lowSeed = 14545
	local midSeed = 12772
	local highSeed = 72346
	
	print(string.format("%-10s | %-6s %-6s %-6s | %-6s %-6s %-6s",
		" / ",
		"wcMin", "wcMid", "wcHigh",
		"reMin", "reMid", "reMax"
	))
	
	for _, iters in pairs({
		{iterL = {-8, -7, 1}, iterH = {-2, 1, 1}},
		{iterL = {8, 7, -1}, iterH = {-2, 1, 1}},
		--[[{iterL = {-2, 1, 1}, iterH = {8, 7, -1}},
		{iterL = {-2, 1, 1}, iterH = {-8, -7, 1}},]]
	}) do
		local lMin, lMax, lStep = table.unpack(iters.iterL)
		local hMin, hMax, hStep = table.unpack(iters.iterH)
		for l = lMin, lMax, lStep do
			for h = hMin, hMax, hStep do
				local wc3min = testRInt(lowSeed, l, h)
				local wc3mid = testRInt(midSeed, l, h)
				local wc3max = testRInt(highSeed, l, h)
				
				local reMin, reMid, reMax = reFormula(l, h)
				print(string.format("%-5d, %-5d | %-7d %-7d %-7d | %-7.1f %-7.1f %-7.1f",
					l, h,
					wc3min, wc3mid, wc3max,
					reMin, reMid, reMax
				))
			end
		end
	end
end
testFormulaInt()

Find min/mid/high seeds for GetRandomInt

nextSeed = 0
seedSteps = 10000000
--[[ Steps must not be a scientific notation or the number becomes a float
and breaks the iterator, it goes on infinitely]]
function step()
	local newMax = nextSeed + seedSteps
	for i = nextSeed, newMax, 1 do
		local min, mid, max = -2^16-3333, 0, 2^16+3333
		SetRandomSeed(i)
		local r = GetRandomInt(min, max)
		if r == min or r == mid or r == max then
			print(string.format("Found seed: %-16d, yields: %-16d", i, r))
			newMax = i
			break
		end
	end
	nextSeed = newMax + 1
	print(string.format("...reached seed: %-16d", newMax))
end
--[[ Integer seeds ]]

--[[ minIntSeed = 14545 ]]
--[[ midIntSeed = 12772 ]]
--[[ highIntSeed = 72346 ]]

SetRandomSeed(14545); print(GetRandomInt(-2^10, 2^9)) -- -1024
SetRandomSeed(12772); print(GetRandomInt(-2^10, 2^9)) -- -1024;-256;512
SetRandomSeed(72346); print(GetRandomInt(-2^10, 2^9)) -- 512

SetRandomSeed(14545); print(GetRandomInt(0, -2^9))
SetRandomSeed(12772); print(GetRandomInt(0, -2^9)) -- 256
SetRandomSeed(72346); print(GetRandomInt(0, -2^9)) -- 512

Max output is offset when using extreme values

-- returns MAX; unless (min-max <= -4294967), then returns (max-SOME_NUMBER)
SetRandomSeed(10702187); print(GetRandomInt(-2^20, 4096))
SetRandomSeed(10702187); print(GetRandomInt(-4294966, 0)) --> 0

SetRandomSeed(10702187); print(GetRandomInt(-4294967, 0)) --> -1

SetRandomSeed(10702187); print(GetRandomInt(-4290871, 4096)) --> 4095

SetRandomSeed(10702187); print(GetRandomInt(-8589934, 0)) --> -2
SetRandomSeed(10702187); print(GetRandomInt(-12884901, 0)) --> -3
SetRandomSeed(10702187); print(GetRandomInt(-17179869, 0)) --> -4
SetRandomSeed(10702187); print(GetRandomInt(-21474836, 0)) --> -5
SetRandomSeed(10702187); print(GetRandomInt(-25769803, 0)) --> -6
SetRandomSeed(10702187); print(GetRandomInt(-30064771, 0)) --> -7
SetRandomSeed(10702187); print(GetRandomInt(-34359738, 0)) --> -8
SetRandomSeed(10702187); print(GetRandomInt(-38654705, 0)) --> -9
SetRandomSeed(10702187); print(GetRandomInt(-42949672, 0)) --> -10
SetRandomSeed(10702187); print(GetRandomInt(-47244640, 0)) --> -11

Code to find the next deviating value (keep calling step()):

nextMin = 0
stepsLimit = 10000000
lastFoundValue = nil
--[[ Steps must not be a scientific notation or the number becomes a float
and breaks the iterator, it goes on infinitely]]
function step()
	local stepsDone = 0
	local found = false
	repeat 
		local min = nextMin
		local max = 0
		SetRandomSeed(10702187)
		local r = GetRandomInt(min, max)
		
		if r ~= lastFoundValue then
			found = true
			
			print(string.format("Found limit: %-16d, yields: %-16d", min, r))
			
			lastFoundValue = r
		end
		
		if nextMin == math.mininteger then
			break
		end
		nextMin = nextMin - 1
		stepsDone = stepsDone + 1
	until found or stepsDone == stepsLimit
	
	print(string.format("...reached limit, nextMin: %-16d", nextMin))
end

getrandomint

Luashine avatar Jul 02 '24 15:07 Luashine