Cleaning up my codes system

Help and Feedback Code Review
#1

I’m formatting my codes like this to practice string manipulation.

I recently made a codes system for my game, but it’s very messy and I assume there’s a better way to do it and clean it up. I’m using Pastebin to store the codes and I use an HTTP request to the raw site to retrieve the codes. I use the following format for my codes:

;:;CodeName;CodeRewardType;CodeRewardDetails.

CodeName is the name of the code :stuck_out_tongue_winking_eye:. CodeRewardType is the type of reward that the player will receive (Ex. Coins). CodeRewardDetails is the ‘amount’ of the reward type that the player receives (Ex. 5). An example is:

;:;T0K3NS;Tokens;15.

This function is what calls the one below. 
local CodesDebounce = false -- This is so a player can't overload the HTTP requests
RedeemCodesRemoteF.OnServerInvoke = function(player, code)
	if CodesDebounce == false then -- Update Code List
		CodesDebounce = true
		local CodeList = HTTPService:GetAsync(URL, false) -- The URL is to an unlisted raw paste on pastebin
		local LastCodeSeparater = 0
		local Stopper = false
		CodesStore = {}
		repeat
			local sTaRt, eNd = string.find(CodeList, ":;:", LastCodeSeparater)
			if sTaRt and eNd then
				if eNd ~= nil then
					LastCodeSeparater = eNd
				end -- These three lines are what goes to the StringStuff function which I'd like to improve.
				local CodeName = string.sub(CodeList, LastCodeSeparater + 1, StringStuff(1, CodeList, LastCodeSeparater))
				local CodeRewardType = string.sub(CodeList, StringStuff(2, CodeList, LastCodeSeparater))
				local CodeRewardDetails = string.sub(CodeList, StringStuff(3, CodeList, LastCodeSeparater))
				local CodeData = {
					name = CodeName,
					rewardType = CodeRewardType,
					rewardDetails = CodeRewardDetails
				}
				wait()
				table.insert(CodesStore, #CodesStore + 1, CodeData)
			else
				Stopper = true
			end
	until Stopper == true
	print("Updated Codes")
		spawn(function()
			wait(60)
			CodesDebounce = false
		end)
	end
	local RedeemedCode
	local Success, Reason
	for i,v in pairs(CodesStore) do
		if v.name == code then
			RedeemedCode = code
			Success, Reason = CodeRewards(player, v.name, v.rewardType, v.rewardDetails) -- CodeRewards is a function to give the rewards to the player. It works fine and I feel no need to improve it.
		end
	end
	if not RedeemedCode then
		return "Invalid Code"
	end
	if Success == "Success" then
		GAEvents:Design(player, "CodeRedemption", RedeemedCode) -- This is a separate module I have to make it simpler to add events to GameAnalytics
		return true 
	elseif Success == "Not Error" then
		return Reason
	elseif Success == "Error" then
		GAEvents:Error(player, 5, "Code Error: "..Reason)
		return "Issue"
	end	
end

That function is not my main concern, this one is. I’m sure there is probably a better way instead of doing all these string.find’s.

local function StringStuff(option, CodeList, LastCodeSeparater)
	if option == 1 then
		if string.find(CodeList, ";", LastCodeSeparater + 1) then
			return string.find(CodeList, ";", LastCodeSeparater + 1) - 1
		else
			return nil
		end
	elseif option == 2 then
		local arg1
		local arg2
		if string.find(CodeList, ";", LastCodeSeparater + 1) then
			arg1 = string.find(CodeList, ";", LastCodeSeparater + 1) + 1
		end
		if string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1)) then
			if string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1) + 2) then
				arg2 = string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1) + 2) - 1
				return arg1, arg2
			else
				return arg1
			end
		end
	elseif option == 3 then
		local arg1
		local arg2
		if string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1)) then
			if string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1) + 1) then
				arg1 = string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1) + 1) + 1
			end
		end
		if string.find(CodeList, ";", string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1))) then
			if string.find(CodeList, ";", string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1) + 1)) then
				if string.find(CodeList, ";", string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1) + 1) + 2) then
					arg2 = string.find(CodeList, ";", string.find(CodeList, ";", string.find(CodeList, ";", LastCodeSeparater + 1) + 1) + 2) - 4
					return arg1, arg2
				else
					return arg1
				end
			end
		end
	end
end

This does function as intended, I just think that it’s an overcomplicated way of doing it. I’ve never really used any string manipulation before, I and thought this might be a good way to practice it.

#2

Why don’t you put the string.find(CodeList, ";", LastCodeSeparater + 1) into a local variable right after you call the function StringStuff, so you can reference it easily compared to repeatedly referencing the entire line each time?

#3

I never thought of that, thanks

#4

There are much much much easier ways to do this. For example, since you are fetching codes from an external site, just make the codes into a JSON string. Therefore, when you fetch the string of codes, you can simply use HttpService:JSONDecode() and get a table of all the codes, what they reward etc etc.

With this approach you can turn your code system into around 2 lines of code.

2 Likes
#5

Oh wow. I’ll look into it, thanks

#6

Made a post to clear up any confusion if you had any :slight_smile:

#7

I seriously can’t thank you enough :joy: