[Gameplay] - With action prediction off, I can give and feed at range (and it never walks into range)

[rezecib]

Bug Submission:

Category: Gameplay

Issue Title: With action prediction off, I can give and feed at range (and it never walks into range)

Issue Description: I play with action prediction off because I personally just don't like it, and in one of the last few hotfixes I stopped walking into range when I tried to give/feed items to things. Instead, if I'm somewhat close to them (~1/2 of a screen away?), it will give/feed them, and otherwise it will say "I can't" and stay in place.

Steps to Reproduce: Turn action prediction off, try to give seeds to a birdcage or feed another player from distance.

[V2C]

Off the top of my head, that sounds like the symptoms of client sending RPCs with parameters in the wrong order.  I am currently unable to repro this with a non-modded server.  Please let us know if you have any more info on this.

[rezecib]

@V2C, Doh! You were spot-on, and it was totally my fault. In Geometric Placement I had to use a structure like this to allow clients to place things that snapped to a grid:

local OldSendRPCToServer = GLOBAL.SendRPCToServerlocal function SendRPCToServer(code, ...)	local arg = {...}	if --[[conditions]] then		--[[modify arg]]	end	OldSendRPCToServer(code, GLOBAL.unpack(arg))endGLOBAL.SendRPCToServer = SendRPCToServer

For some reason, I guess unpack didn't always preserve the order or something, since changing it to this seemed to fix it:

local OldSendRPCToServer = GLOBAL.SendRPCToServerlocal function SendRPCToServer(code, ...)	local arg = {...}	if --[[conditions]] then		--[[modify arg]]		OldSendRPCToServer(code, GLOBAL.unpack(arg))	else		OldSendRPCToServer(code, ...)	endendGLOBAL.SendRPCToServer = SendRPCToServer

[simplex]

@V2C, Doh! You were spot-on, and it was totally my fault. In Geometric Placement I had to use a structure like this to allow clients to place things that snapped to a grid:

local OldSendRPCToServer = GLOBAL.SendRPCToServerlocal function SendRPCToServer(code, ...)	local arg = {...}	if --[[conditions]] then		--[[modify arg]]	end	OldSendRPCToServer(code, GLOBAL.unpack(arg))endGLOBAL.SendRPCToServer = SendRPCToServer

For some reason, I guess unpack didn't always preserve the order or something, since changing it to this seemed to fix it:

local OldSendRPCToServer = GLOBAL.SendRPCToServerlocal function SendRPCToServer(code, ...)	local arg = {...}	if --[[conditions]] then		--[[modify arg]]		OldSendRPCToServer(code, GLOBAL.unpack(arg))	else		OldSendRPCToServer(code, ...)	endendGLOBAL.SendRPCToServer = SendRPCToServer

unpack does preserve order. Were you replacing elements in arg with nil and increasing the size of the table? unpack returns the array part of the table as a list of elements. Simply adding nils does not change what is considered to be in the array part, but once the table grows enough (to the next power of 2, to be exact) the array and hash parts undergo resizing, and what was previously in the array part may get put in the hash part, being no longer returned by unpack.

[rezecib]

@simplex, Well, I didn't change when the arguments were being modified. The only change, as far as I can tell, is that if it doesn't modify them, then it uses "..." instead of unpack(arg), whereas before it always used unpack(arg) regardless of whether arg got modified or not. I guess that the "local arg = {...}" part could also be the culprit, though. If you're curious about the specifics, here's the whole modification, including failed attempts and monologue:

-- #rezecib Added this to fix planting on clients in DST-- This feels really hackish...... but there doesn't seem to be a better way to do it,--  since this is directly called from the monstrous PlayerController functionslocal OldSendRPCToServer = GLOBAL.SendRPCToServerlocal function SendRPCToServer(code, ...)	local arg = {...}	-- for k,v in pairs(arg) do print(k,v) end	--#rezecib not the best solution to the CTRL check here, but... no good options 	if (not TheInput:IsKeyDown(KEY_CTRL))		and (code == GLOBAL.RPC.ActionButton or code == GLOBAL.RPC.RightClick)		and arg and arg[1] == GLOBAL.ACTIONS.DEPLOY.code then		local ThePlayer = GLOBAL.ThePlayer		if not (ThePlayer and ThePlayer.replica and ThePlayer.replica.inventory and			ThePlayer.replica.inventory.classified and ThePlayer.replica.inventory.classified:GetActiveItem()			and (ThePlayer.replica.inventory.classified:GetActiveItem():HasTag("wallbuilder") 				or ThePlayer.replica.inventory.classified:GetActiveItem():HasTag("groundtile"))) then			arg[2] = arg[2]+.25-(arg[2]+.25)%.5			arg[3] = arg[3]+.25-(arg[3]+.25)%.5		end		OldSendRPCToServer(code, GLOBAL.unpack(arg))	else		OldSendRPCToServer(code, ...)	end	-- OldSendRPCToServer(code, GLOBAL.unpack(arg))	-- OldSendRPCToServer(code, ...)endGLOBAL.SendRPCToServer = SendRPCToServer

[simplex]

@simplex, Well, I didn't change when the arguments were being modified. The only change, as far as I can tell, is that if it doesn't modify them, then it uses "..." instead of unpack(arg), whereas before it always used unpack(arg) regardless of whether arg got modified or not. I guess that the "local arg = {...}" part could also be the culprit, though. If you're curious about the specifics, here's the whole modification, including failed attempts and monologue:

Well... that really doesn't make sense to me. Using unpack or not should make no difference. I'm glad you fixed it, I just don't understand how.

[V2C]

SendRPCToServer was updated to support nil arguments, whereas {...} and unpack only returns up to the first nil, so some arguments were being lost in this case.