Am I going about making this combat system right?

Basically the following code is the ‘front end’ of my combat script, I’m really confused if this is a good way of handling tool use-age.

if inputObj.UserInputType == Enum.UserInputType.MouseButton1 then	
			if inputModule.checkDebounce('running') or inputModule.checkDebounce('climbing') then
				return
			end
			
			if not character:FindFirstChildOfClass('Tool') then
				if not inputModule.checkDebounce('toolSwapDelay') then
					inputModule.stopRunning()
					-- Punch
					game.ReplicatedStorage.Events._useTool:FireServer('punch')
				end
			else
				local SelectedTool = character:FindFirstChildOfClass('Tool')
				
				if not inputModule.checkDebounce('toolSwapDelay') then
					inputModule.stopRunning()
					game.ReplicatedStorage.Events._useTool:FireServer('useTool')
				end
			end
		end

The key lines here are regarding the remotes, I don’t know if I should tell the server I have x sword or if I should transmit any information regarding what tool I’m trying to use at all? Also let’s say I do fire said remote, how do I then know if the server says sure you can attack! then apply a debounce? should the server fire back to the client afterwards to make those adjustments?

This script is fine as long as everything gets sanity checked on the server-side. Be warned that your checks on the client can be bypassed by exploiters.

The only use for firing back to the client is to confirm the move happened so the client can do things like adjust their UI for cooldowns. You’d risk desyncs if you adjust things before the server confirms what happened.

1 Like

Okay to break everything down, the most basic version of this script would be simply firing a remote to the server everytime you press mousebutton1, everything including determining the tool, etc should be done on the server? So passing strings to try and determine what action the player is doing may not be the best way to go about things? Curious what you think

Yes. The only thing the server should be given is the move the client is attempting to preform.

In certain cases, you might want to skip ahead and preform calculations before any server confirmation, but that’s only in FPS games where you need rapid-fire guns.

1 Like
game.ReplicatedStorage.Events._useTool.OnServerEvent:Connect(function(player, actionContext)
	-- Determine what the 'player' is trying to do here.
	
	local character = player.Character
	local selectedTool
	
	-- first lets determine what tool you're trying to use :)
	
	if character:FindFirstChildOfClass('Tool') then
		selectedTool = character:FindFirstChildOfClass('Tool')
	else
			--// PUNCH LOGIC \\--
		if actionContext == 'M1' then
			combatModule.MouseButton1_Attack()
		end
		
		if actionContext == 'M2' then
			combatModule.MouseButton2_Attack()
		end
	end
	
	if selectedTool then
		
		-- determine what to do!! :)
		
		if selectedTool:GetAttribute('toolType') == 'weapon' then
			if actionContext == 'M1' then
				combatModule.MouseButton1_Attack()
			end

			if actionContext == 'M2' then
				combatModule.MouseButton2_Attack()
			end
		end
		
		if selectedTool:GetAttribute('toolType') == 'ability' and actionContext == 'M1' then
			warn('no logic for abilities yet :( come back later')
		end
		
		if selectedTool:GetAttribute('toolType') == 'food' then
			warn('no logic for food!')
		end
		
		if selectedTool:GetAttribute('toolType') == 'potion' then
			warn('no logic for potions either!')
		end
		
		if selectedTool:GetAttribute('toolType') == 'artifact' then
			warn('no logic for artifiacts yet c:')
		end
		
		-- more logic here
	end
end)

is this a fair way to handle this on the server? basically grab the weapon from the character & “context” basically pass a string being m1 or m2 etc and then based on that and attributes determine whats happening & run the respective function.

would you go about this a different way? I can’t really think of any other way to do this.

There’s not much to improve here. You could change the punch logic to take up 3 less lines but that’s about it.