Any Tips to Improve my PlayerClass?

Is there any way to improve my Class Module? Just wondering what I can do to make it more efficient. Also, don’t mind CFramework on the top. Just my custom framework. :wink:

Another thing to mention, CharacterAutoReloads will be off.

local CFramework = require(game:GetService("ReplicatedStorage"):WaitForChild("Shared"):WaitForChild("CFramework"))

local PlayerClass = CFramework:New({}, "Class", "PlayerClass")
PlayerClass.__index = PlayerClass


function PlayerClass.new(Player, State, Level, Inventory)
	--[[
	self.Name is self Explantory
	self.PlayerReference is a Reference to the Player, Incase I create future methods
	Two Different Possible Status: "IN_GAME" and "DEAD"
	self.Level is basically a stat
	self.Inventory is a table of the user's Inventory.
	
	]]--
	local self = setmetatable({}, PlayerClass)
	self.Name = Player.Name
	self.PlayerReference = Player
	self.State = State or nil
	self.Level = Level.L or 1
	self.Inventory = Inventory
	
	return self
end

function PlayerClass:UpdateStatus(Status)
	-- Must be Called on to occur, doesn't happen randomly.
	self.State = Status
	return self.State 
end

function PlayerClass:UpdateLevel()
	
	-- Occurs Every Minute via While True Do
	if	self.Level.XPRequired <= self.Level.CurrentXP then
		self.Level.L += 1
	end
	if self.Level.L >= 100 then
		self.Level.Prestige += 1
                self.Level.L = 1

	end
	
end

function PlayerClass:IsDead()
	wait(1)
	self.PlayerReference:LoadCharacter()
end

function PlayerClass:UnboxedItem(item)
	for i, v in pairs(self.Inventory) do
		if item == v then
			v += 1
		elseif item ~= v then
			table.insert(self.Inventory, #self.Inventory+1, item)
			
		end
	end
end
return PlayerClass
2 Likes

Looks good, except don’t call while true to update XP, call it every time XP Value is changed to improve the player’s experience

2 Likes

I’d say the layout is perfectly fine, but there are a few areas for improvement.

  1. Naming functions
function PlayerClass:IsDead()
	wait(1)
	self.PlayerReference:LoadCharacter()
end

Is the first instance I see where naming could be changed a bit (this feels a bit nitpicky) but can help other programmers or even yourself reading coding in the future. Remember, optimise code for reading not for writing. The problem here is that you named the function IsDead() however it returns nothing, if I saw IsDead being called in a script i’d expect it to return a true or false value. From the looks of it you respawn the play, so maybe renaming the function to :Respawn(). Besides that single function the rest of the naming seems to be in-order.


2) Using waits

It’s time for me to go on another rant on how bad wait() is, but instead of doing that i’ll redirect you to a thread that helps a lot with explaining the pros and cons with wait. A thread that you can find here.

And if you DO need to use wait, you can use the FastWait module by clonetrooper.

4 Likes

This point is irrelevant as OP is not using wait() but wait(n).

@OP:

function PlayerClass:UnboxedItem(item)
	for i, v in pairs(self.Inventory) do
		if item == v then
			v += 1
		elseif item ~= v then
			table.insert(self.Inventory, #self.Inventory+1, item)
			
		end
	end
end

You can omit elseif item ~= v and replace it with just else. You also don’t need to pass a extraneous argument for table.insert as the second argument, instead just pass in the value you want to add. It will automatically add that value to the end of the table which is what your code is doing.

if item == v then
   v += 1
else
     table.insert(self.Inventory, item)
end

This function is supposed to indicate whether the player is dead, but you’re just loading the player’s character again.

function PlayerClass:IsDead()
	wait(1)
	self.PlayerReference:LoadCharacter()
end

Either change the name for readability or have your function do what it is supposed to do:

function PlayerClass:IsDead()
    return self.PlayerReference.Character.Humanoid.Health == 0
end

A better way to create tables in your constructor function would be appending the keys when the table is being created, not after it has been created:

local self = setmetatable({
     Name = Player.Name, 
     PlayerReference = Player
	 State = State or nil
	 Level = Level.L or 1
	 Inventory = Inventory
}, PlayerClass)

	
return self
3 Likes

This point is irrelevant as OP is not using wait() but wait(n) .

In the thread I linked that related to wait() it mentioned how wait(n) can wait longer than intended, which is why I provided a FastWait module, which makes the wait(n) function much more accurate in situations where the game may not be performing at optimal performance.

Quote from this thread about avoiding wait

A major issue is that wait() (and also wait(n) ) can wait for MUCH longer than you want it to. The more you call wait() , the longer it will take. wait() can easily last seconds , leading to a very laggy game experience.

2 Likes

wait(n) doesn’t necessarily become inaccurate if the game isn’t performing optimally, the amount of threads needed to be resumed also play a factor.

There is no reason to use an external module, just use this function which uses Heartbeat:

local Heartbeat = game:GetService("RunService").Heartbeat

local function HeartbeatWait(yieldTime)
    yieldTime = typeof(yieldTime) == "number" and yieldTime or 0 
    local dtPassed = 0

    while true do
          if dtPassed >= yieldTime then
              break
         end
         dtPassed += Heartbeat:Wait() 
    end
end

The read you linked is about wait(), not wait(n).

Thank you for that, really didn’t know I was adding unnecessary arguments to else.

I agree, RunService is completely Server Side, and tbh RunService.HeartBeat:Wait() is more faster and easier to use, than regular wait(). However, wait(n) is literally the same thing as RunService.HeartBeat:Wait(n).

Also, wait()/wait(n) will only become inaccurate IF:

  • game is not optimized (like are you kidding me it’s not that hard to optimize your game, adopt me is such a large-scale game yet I can get 60-90 fps using a garbage laptop)

I don’t see wait(n) causing any problems, it’s just that Heartbeat:Wait() is faster than wait().

edit: fyi inaccurate wait() is caused by a laggy experience, so really it’s not about that causing lag.

wait(n) is not the same thing as RunService.Heartbeat:Wait(n). Roblox servers down will cause no inaccuracy to wait, and it isn’t a void function, it returns the time it yielded the thread for.

wait is not only affected by lag, but also the current amount of threads to be resumed.

1 Like

What?

Bro, RunService works perfectly on clientside

wait() is inaccurate while RunService allows you to perfectly time anything.

This is incorrect, wait() isn’t necessarily inaccurate and RunService doesn’t allow you to perfectly time anything, it is just more reliable.

If wait is affected by lag, so is Heartbeat.Wait affected by lag.
We are supposed to work with the delta time. It should not matter whether we use wait or Heartbeat.Wait

1 Like

Heartbeat wait is MUCH more reliable than wait, it is put into the primary thread and has a higher frequency than wait, which is just bound to the 30hz frequency while Heartbeat wait can even exceed more than the 60hz frequency if used on the client with more than 60 FPS.

If Heartbeat is affected, it will not be that noticeable as compared to wait. Your point is irrelevant, delta time is not used for yielding the thread.

The wait is perhaps not as reliable (as in not waiting the specified time as accurately as intended), yet the time that it yielded is returned. Say for example we want to increment a value by 1 per second.

The wait version:

local v = 0
while 1 do
  local dt = wait(1)
  v += dt
end

The Heartbeat.Wait version:

local Heartbeat = game['Run Service'].Heartbeat
local v = 0
while 1 do
  local dt = 0
  repeat
    dt += Heartbeat:Wait()
  until dt >= 1
  v += dt
end

As you can imagine, not only is wait much less cumbersome to use, it also gets the job done.
The only difference is that under extreme lag, the Heartbeat.Wait version can update the value 1/60 second faster.

1 Like

Why are you spreading false information? Heartbeat.Wait will resume much faster than wait, not only under “extreme lag”.

Heartbeat.Wait also returns the time it yielded the thread for which proves your entire point incorrect. We’re talking about wait(), not wait(n).

print(game:GetService("RunService").Heartbeat:Wait()) -- 0.028019992634654

PS: I concluded a benchmark on 10 FPS on the client, testing out both. Here are the results:

image
image

Left is how much wait yielded for while the right is how much heartbeat yielded for.

You can clearly see that Heartbeat is much faster and superior to use.

No one said anything about Heartbeat.Wait not returning the time yielded. I am just saying how it is much more intuitive to use wait instead of Heartbeat.Wait if the delta time is to be accounted for (we should almost always account for the delta if you still haven’t noticed by now), and it is still affected by lag just as much as wait do.
wait:


Heartbeat.Wait:

They are functionally equivalent with a minor difference. You SHOULD account for the delta time anyways.

Sure in loops where we do not have to concern ourselves with the delta time, it is better suited to use Heartbeat.Connect, just like what this page have told us to.

Returning faster or not is not the concern here.

It is functionally equivalent. There is a default value set. Even by switching the Heartbeat:Wait() to wait() there would have achieved the exact same effect.

If wait is such a bad utility it would have been deprecated and replaced.

Returning faster or not is not the concern here.

That is my point.

It is functionally equivalent. There is a default value set. Even by switching the Heartbeat:Wait() to wait() there would have achieved the exact same effect. If wait is such a bad utility it would have been deprecated and replaced.

That isn’t my point, my point was just to address how Heartbeat.Wait can be faster and is much reliable than wait and they aren’t equivalent in terms of accuracy and yielding, this is an false claim.

@Feedekaiser

PS:

They are functionally equivalent with a minor difference. You SHOULD account for the delta time anyways.

This is completely irrelevant, we are talking about yielding, not using delta time. You don’t use delta time for yielding unless you want to create your own function.

If wait is such a bad utility it would have been deprecated and replaced.

No, wait should be used when it needs to be and you don’t care about exact accuracy. I never said it was a bad function, it should be used when it needs to be.

Sure in loops where we do not have to concern ourselves with the delta time, it is better suited to use Heartbeat.Connect , just like what this page have told us to.

Have you even read the properly? You’re creating an unnecessary thread (Heartbeat.Connect) and that is definitely not needed for yielding: you need to yield the current thread, not the one created by Heartbeat.Connect.

That page has never stated to use Heartbeat.Connect for yielding and hence, your point is completely false.

(we should almost always account for the delta if you still haven’t noticed by now), and it is still affected by lag just as much as wait do.

Like I said, Heartbeat.Wait isn’t affected by “lag” as wait, it is much more reliable and accurate when compared to wait, if wait is affected, sure Heartbeat.Wait will be but since it isn’t bound to the 30hz frequency, the affect won’t be as bad as compared to wait and is mostly unnoticeable, refer to my benchmark above.

There is no point in arguing pointlessly with false claims.

Thanks for that, didn’t really know that you could run RunService on client, I’ve had a few issues where, I thought that the cause of it was RunService. That’s why I’ve come to believe that RunService is only server side.

That test has many different issues. First of all, those wait() yield times only go that low if your running at 10 fps. Those numbers would be dramatically similar if you were running at 60 fps. Also, good optimization reduces the amount of lag in the server, causing higher fps in the client. In a 60 fps benchmark, wait() and Heartbeat are dramatically similar. However, if the given client FPS gets too low or too high(Roblox FPS Unlocker I’m looking at you), Heartbeat will prevail by a large margin.

As most of roblox’s games support optimization, and roblox’s main playerbase are kids, I don’t think getting low fps or getting high fps will be common. For games like adopt me, wait() wouldn’t cause such a huge problem, for obvious reasons. However, I do agree heartbeat would be extremely useful in games such as Arsenal or any other kind of shooters.

while I do agree with you that HeartBeat:Wait() is much, MUCH faster than wait() in most situations, the evidence you used to provide usually wouldn’t happen in most games. (most laptops and computer can easily run games at 60 fps), phones can run adopt me at 60 fps. Not necessarily denying Heartbeat’s speed, but the benchmark doesn’t really apply to that much people, and games. If something like fps caused an issue, the game itself would already be hard to play.

Any wait() method from Luau is just bad.

Kampfkarren already said in this tweet, that he doesn’t even think any wait should be used no matter what, that’s why there’s those modules like BetterWait.