Ocean fishing reeling client prediction issues (extremely common animation issues)

Usually, holding the reel action input tends to make things work more smoothly since at that point it's up to the server. However, when it's instead pressed repeatedly, quickly or not, the following can happen:

1. Client player is able to input the reel action without any sort of delay, unlike on the server.
2. Due to 1, the client player can go from "oceanfishing_idle" or "oceanfishing_reel" to a generic "idle_loop" by inputting the reel action too fast, but it can also happen on its own sometimes due to nature of the update function on the client reel state.
3. Reel tension animations can be overridden by the predicted animations, resulting in delayed visual feedback (sound is always on point due to being server-sided, hearing the sound but not the animation isn't latency, it's prediction getting in the way), which is bad for when the line is tense and might break. The player may also get stuck in an incorrect predicted looped animation due to this.
4. Stop (retrieving the hook without anything caught on it), catch (catching fish/sunk object) and snap (fishing line broke) animations can also be overridden. Although this is more cosmetic, it still happens very often.

Some of these might happen even when holding the input, if it was clicked too recently that is.

---------------------------------------------------------------------------------------------------------------------------------

Here's what I tried, by tweaking things only in SGwilson_client. Testing included both as server client host (caves enabled of course) and as a client with ~200ms:

• Added "oceanfishing_idle" state to the server_states table of the oceanfishing_reel state. This is because we can predict that 99% of the time, we'll go from reeling back to idle, and viceversa. This also lets the client chill in the blank idle state (until the input is pressed again), letting the server handle it from there. If the server never reached either idle or reel, the client will time out after two seconds as usual.

• Removed nil buffered action bit of the onupdate code that sends us to a generic idle state (only a timeout should do this, because we have no predicted oceanfishing_idle state to go back to), shown below:

-- don't exit this state prematurely, let timeout handle it if it really comes to it
-- wait until our state matches the server or (as failsafe), we entered some other busy state (like oceanfishing_catch)

onupdate = function(inst)
	if inst.sg:ServerStateMatches() then
		if inst.entity:FlattenMovementPrediction() then
			inst.sg:GoToState("idle", "noanim")
		end
	--elseif inst.bufferedaction == nil then
	--	inst.sg:GoToState("idle")
	elseif inst:HasTag("busy") then
		inst.sg:GoToState("idle", "noanim")
	end
end

(an alternative would be to predict the idle animation and go to idle with no animation, or to have the generic idle state check if we're supposed to be in a fishing idle, but either of these might still run into issues due to the nature of this action)

• Do the following with the onenter function (as well as add an onexit function), and with the OCEAN_FISHING_REEL action handler. All in one bit of code with some comments to simplify explaining it here as it'd be too long:

-- leaving commented out print lines for more context, structure it as you see fit
ActionHandler(ACTIONS.OCEAN_FISHING_REEL,
	function(inst, action)
		local fishable = action.invobject ~= nil and action.invobject.replica.oceanfishingrod:GetTarget() or nil
		if fishable ~= nil and fishable:HasTag("oceanfishing_catchable") then
			--print("Target is catchable, we shouldn't have gotten here. Investigate?) -- this isn't common but I had it print
		elseif fishable ~= nil and fishable:HasTag("partiallyhooked") then
			return "oceanfishing_sethook"
		elseif inst.ocean_reel_repeat_delay ~= nil then
			--print("Too soon to retry reeling.") -- expected
		elseif inst:HasTag("fishing_idle") then
			if inst:HasTag("busy") or inst.sg:HasStateTag("busy") then
				--print("We're in a busy state, can't reel.") -- this isn't common but I had it print
			else
				return "oceanfishing_reel"
			end
		end
		return nil
	end)

-------------------------------------------

local function RemoveDelay(inst)
	inst.ocean_reel_repeat_delay = nil
end

onenter = function(inst)
	-- shouldn't be needed, but just in case
	if inst.ocean_reel_repeat_delay ~= nil then
		inst.ocean_reel_repeat_delay:Cancel()
	end

	-- prevent clients from spamming the action way too fast
	-- helps against event desync and animations mistmatching too easily
	-- this could be a bit more intricate, but it worked fine even with ~200ms of latency (makes sense given the delay is 0.3 seconds)
	inst.ocean_reel_repeat_delay = inst:DoTaskInTime(TUNING.OCEAN_FISHING.REEL_ACTION_REPEAT_DELAY, RemoveDelay)

	inst.components.locomotor:Stop()

	local rod = inst.replica.inventory:GetEquippedItem(EQUIPSLOTS.HANDS)
	rod = (rod ~= nil and rod.replica.oceanfishingrod ~= nil) and rod.replica.oceanfishingrod or nil -- cache replica component instead of rod
	local target = rod ~= nil and rod:GetTarget() or nil

	-- correctly check for fishinghook tag
	-- oceanfishinghook component only exists on the server
	--if target == nil or target.component.oceanfishinghook ~= nil or rod:IsLineTensionLow() then
	if target == nil or target:HasTag("fishinghook") or rod:IsLineTensionLow() then
		if not inst.AnimState:IsCurrentAnimation("hooked_loose_reeling") then
			inst.AnimState:PlayAnimation("hooked_loose_reeling")
		end
	elseif rod:IsLineTensionGood() then
		if not inst.AnimState:IsCurrentAnimation("hooked_good_reeling") then
			inst.AnimState:PlayAnimation("hooked_good_reeling")
		end
	elseif not inst.AnimState:IsCurrentAnimation("hooked_tight_reeling") then
		inst.AnimState:PlayAnimation("hooked_tight_reeling")
	end

	inst:PerformPreviewBufferedAction()
	inst.sg:SetTimeout(TIMEOUT)

	-- haven't tested without this and the onexit function
	-- with some understanding of how this works, and with all the changes combined
	-- I never ran into animation issues with reel actions again, but both of these might be unneeded
	inst.entity:SetIsPredictingMovement(false)
end

onexit = function(inst)
	inst.entity:SetIsPredictingMovement(true)
end

I apologize if this is a bit too long or obscure in some bits while other bits aren't, I wanted to be through with the report, but not make it massive by putting multiple code snippets with vanilla vs tweaked code or so. I'm attaching the client-sided script I made for further reference, in case that's more readable than this.

Edit: the "oceanfishing_reel" state in SGwilson.lua could also use get an onupdate function to update sounds and animations in the middle of reeling, to make it more responsive.

modmain.lua