Limelight Forums

(Aug 26, 2015, 03:36 PM)Voluptious link Wrote: [ -> ][quote author=Rayts5 link=topic=1167.msg12981#msg12981 date=1440595827]
Note, careful on your variables.
local str = PackageTable(ply:RollDice(args[1], args[2], args[3]))
net.WriteString(str)

Is the same as

net.WriteString(PackageTable(ply:RollDice(args[1], args[2], args[3])))

I know that the above looks better since you store a string, but you are performing 1 extra unnecessary operation.

Its easier to read and debug if you do it this way, I do things like this aswell
[/quote]

It is, but you are moving a var to a register which is slower than just calling the retrieved thing itself. There is no need to store more stuff to our base pointer.

(Aug 26, 2015, 03:38 PM)Rayts5 link Wrote: [ -> ][quote author=Voluptious link=topic=1167.msg12983#msg12983 date=1440596209]
[quote author=Rayts5 link=topic=1167.msg12981#msg12981 date=1440595827]
Note, careful on your variables.
local str = PackageTable(ply:RollDice(args[1], args[2], args[3]))
net.WriteString(str)

Is the same as

net.WriteString(PackageTable(ply:RollDice(args[1], args[2], args[3])))

I know that the above looks better since you store a string, but you are performing 1 extra unnecessary operation.

Its easier to read and debug if you do it this way, I do things like this aswell
[/quote]

It is, but you are moving a var to a register which is slower than just calling the retrieved thing itself. There is no need to store more stuff to our base pointer.
[/quote]

[Image: Jackie-Chan-WTF.jpg]

What are you on about. All these words. What's a 'var', can you eat it?

(Aug 26, 2015, 03:30 PM)Rayts5 link Wrote: [ -> ]... What? Why are you utilizing two network variables for this? It does not give sense any sense, you are making the server handle both net.Receive and net.Start lol.

Simply make 1 (or 2 if you want to) concommands on the server-side and handle the rolling and such from there. There is absolutley no need to store a network variable on the player, just store a variable on the player instead.

Note, careful on your variables.
local str = PackageTable(ply:RollDice(args[1], args[2], args[3]))
net.WriteString(str)

Is the same as

net.WriteString(PackageTable(ply:RollDice(args[1], args[2], args[3])))

I know that the above looks better since you store a string, but you are performing 1 extra unnecessary operation.

Also, in your GetNearbyPlayers function, just store 1 table instead before the function, otherwise you will keep adding tables in the stack which is generally a bad idea (unless you want it to overflow lol, I do not believe Lua had a garbage collector). At start, clear the table and then add the players to the table.

I'll go at it piece by piece since there are a few things, one thing first by no means in this script perfect and I pretty happy someone actually decided to say there was something wrong with it. Because it is by no means perfect. Also if I misunderstand you throughout the post my bad, just correct me again ^^

I presume you're talking about the net library when you say I'm handling two network variables server side. There is a if SERVER check so net strings at cached on the server and the receivers are set up on the client side. So I'm not really sure what you are getting at there. I think I have a rough idea looking at it again. I think you're talking about the use of two net strings which I agree is completely unnecessary (mentioned in comments iirc) that's due to me rethinking how it was going to be done mid way, but it doesn't make much difference performance wise. I'll make the change anyways.

I'm not networking any variables on the player, they are being index to the player. This is done so we can get there last roll if for some reason chat is cleared or the player intended to receive the role didn't see it. Literally what you describe is done next. There are two console commands, (there could be one but I was lazy, check comments ^^) that handle the rolling server side. So as you said I am storing the variable on the player and it is not being networked.

There is actually another mistake I noticed thanks for pointing it out thought I changed it. You currently cannot see who rolled, I thought I add the players name to the start. Either way you are correct here and it servers no other use than being easier to change and read.

On the note of overflowing the stack with the creation of tables. We don't have to worry about shit like that fortunately as lua runs it's gc periodically getting rid of variables it can no longer access (iirc? Correct me if need be). Though saying that your way would be better, as far I'm aware though there will be no noticeable performance difference.

I don't have a great deal of knowledge in low level programming, I'd love to learn and will probably start more when I get back. Thanks for the heads up with your points I'll make a second version incorporating your ideas. I'll post it I'm the next few days depending on timings etc..

Sorry if this came off the wrong way, this was all typed on my phone so it's hard to read back and make changes. Anyways thanks for the help I'll post a new version soonish

(Aug 26, 2015, 04:42 PM)Lord Octagon link Wrote: [ -> ][quote author=Rayts5 link=topic=1167.msg12984#msg12984 date=1440596281]
[quote author=Voluptious link=topic=1167.msg12983#msg12983 date=1440596209]
[quote author=Rayts5 link=topic=1167.msg12981#msg12981 date=1440595827]
Note, careful on your variables.
local str = PackageTable(ply:RollDice(args[1], args[2], args[3]))
net.WriteString(str)

Is the same as

net.WriteString(PackageTable(ply:RollDice(args[1], args[2], args[3])))

I know that the above looks better since you store a string, but you are performing 1 extra unnecessary operation.

Its easier to read and debug if you do it this way, I do things like this aswell
[/quote]

It is, but you are moving a var to a register which is slower than just calling the retrieved thing itself. There is no need to store more stuff to our base pointer.
[/quote]

[Image: Jackie-Chan-WTF.jpg]

What are you on about. All these words. What's a 'var', can you eat it?
[/quote]

Low level programming, don't you worry. Wink

(Aug 26, 2015, 05:08 PM)Spy link Wrote: [ -> ][quote author=Rayts5 link=topic=1167.msg12981#msg12981 date=1440595827]
... What? Why are you utilizing two network variables for this? It does not give sense any sense, you are making the server handle both net.Receive and net.Start lol.

Simply make 1 (or 2 if you want to) concommands on the server-side and handle the rolling and such from there. There is absolutley no need to store a network variable on the player, just store a variable on the player instead.

Note, careful on your variables.
local str = PackageTable(ply:RollDice(args[1], args[2], args[3]))
net.WriteString(str)

Is the same as

net.WriteString(PackageTable(ply:RollDice(args[1], args[2], args[3])))

I know that the above looks better since you store a string, but you are performing 1 extra unnecessary operation.

Also, in your GetNearbyPlayers function, just store 1 table instead before the function, otherwise you will keep adding tables in the stack which is generally a bad idea (unless you want it to overflow lol, I do not believe Lua had a garbage collector). At start, clear the table and then add the players to the table.

I'll go at it piece by piece since there are a few things, one thing first by no means in this script perfect and I pretty happy someone actually decided to say there was something wrong with it. Because it is by no means perfect. Also if I misunderstand you throughout the post my bad, just correct me again ^^

I presume you're talking about the net library when you say I'm handling two network variables server side. There is a if SERVER check so net strings at cached on the server and the receivers are set up on the client side. So I'm not really sure what you are getting at there. I think I have a rough idea looking at it again. I think you're talking about the use of two net strings which I agree is completely unnecessary (mentioned in comments iirc) that's due to me rethinking how it was going to be done mid way, but it doesn't make much difference performance wise. I'll make the change anyways.

I'm not networking any variables on the player, they are being index to the player. This is done so we can get there last roll if for some reason chat is cleared or the player intended to receive the role didn't see it. Literally what you describe is done next. There are two console commands, (there could be one but I was lazy, check comments ^^) that handle the rolling server side. So as you said I am storing the variable on the player and it is not being networked.

There is actually another mistake I noticed thanks for pointing it out thought I changed it. You currently cannot see who rolled, I thought I add the players name to the start. Either way you are correct here and it servers no other use than being easier to change and read.

On the note of overflowing the stack with the creation of tables. We don't have to worry about shit like that fortunately as lua runs it's gc periodically getting rid of variables it can no longer access (iirc? Correct me if need be). Though saying that your way would be better, as far I'm aware though there will be no noticeable performance difference.

I don't have a great deal of knowledge in low level programming, I'd love to learn and will probably start more when I get back. Thanks for the heads up with your points I'll make a second version incorporating your ideas. I'll post it I'm the next few days depending on timings etc..

Sorry if this came off the wrong way, this was all typed on my phone so it's hard to read back and make changes. Anyways thanks for the help I'll post a new version soonish
[/quote]

It did not come out the wrong way at all. ^^ I might have been too sharp in my first comment too, in terms of reviewing the code. What I wanted to say, was the fact that it is not necessary to use the Net library at all as it can simply be stored on the player variable; concommands are usually handled server-side after all. I can however give a super thumbs up for continuing to improve the code. Some would see that it works, and then just carry on, despite its looks.

I am not sure about how good the current gc is in Lua, but you are correct that a garbage collector removes variables which points to unused memory.

If you need help with lower level programming, also, I can help you with it. Ever since I got into low level programming (several months ago), I have started to see everything at that level. Which, is also quite useful when coding C++.

The net library use was purely so it could be outputted into chat for others to see (limited to those around) that was the only way of doing off the top of my head. I'm sure this gamemode has some built in functions to remove the need for mine, unfortunately I don't have any experience with this gamemode Confused

. If you have any other suggestions I'll be happy to hear em, thanks so far. Ill probably take you up on that offer (low level help) when I get back.

(Aug 26, 2015, 05:54 PM)Spy link Wrote: [ -> ]The net library use was purely so it could be outputted into chat for others to see (limited to those around) that was the only way of doing off the top of my head. I'm sure this gamemode has some built in functions to remove the need for mine, unfortunately I don't have any experience with this gamemode . If you have any other suggestions I'll be happy to hear em, thanks so far. Ill probably take you up on that offer (low level help) when I get back.

Aaah, I see why you wanted to use the Net library then. You can grab all players in X radius around you, and SendLua the chat command too.

EDIT: Christ I am tired, I missed the "else" and saw your net.Receive which means that it is handled on the client *nodnod*, it must have been due to the code tags on the forum... Thingy. Alright, that changes everything. Personally, I wouldn't add a network string for that task, but it can be done however. You can also turn it into one network string. The reason for why I wouldn't, is because the network library can handle up to a size of 64kb, aka 64000 characters. You won't need 64kb for that task. Tongue

(Aug 26, 2015, 06:03 PM)Rayts5 link Wrote: [ -> ]Aaah, I see why you wanted to use the Net library then. You can grab all players in X radius around you, and SendLua the chat command too.

EDIT: Christ I am tired, I missed the "else" and saw your net.Receive which means that it is handled on the client *nodnod*, it must have been due to the code tags on the forum... Thingy. Alright, that changes everything. Personally, I wouldn't add a network string for that task, but it can be done however. You can also turn it into one network string. The reason for why I wouldn't, is because the network library can handle up to a size of 64kb, aka 64000 characters. You won't need 64kb for that task.

Meh if I were to send a 64kb string there is a pretty large chance I'll overflow the net channel. Won't be a problem either way, latest version limits rolls. Using SendLua I'm this case will probably be more effort as it will lead to a lua injection. Anyways I implemented your suggestions regarding optimisations as well as fixing the issues are I mentioned earlier. Thanks of the help I credited you ^^.

Code:
-- Created by Spy (steamcommuntiy.com/id/Snappey)

-- Credit to Rayts5 (Small optimizations here and there)

-- TODO:

-- add errors messages for cooldown and too many rolls

-- Create chat commands, use gamemode chat command system

-- 

if SERVER then

    util.AddNetworkString("PlayerRolled")

    local str = ""

    local function PackageResult(tbl)

    str = "" -- so we cant add other strings to it

        for k,v in pairs(tbl) do 

            str = str .. "[ Dice #"..k.."] Rolled " .. v.res .. " (min: " .. v.min ..", max: " .. v.max..") /n" -- newline maybe idk

        end

        return str

    end

    local tbl = {} -- credit to ratys5 pretty sure this does slightly optimize

    local function GetNearbyPlayers(pos)

        tbl = {} -- afaik his should nit create a new local var each time and use the one created outside the function, credits to rayts5

        for k,v in pairs(ents.FindInSphere(pos, 240)) do -- Think 240 is distance of talking 

            if !v:IsPlayer() then return end

            table.insert(tbl, v)

        end

        return tbl

    end

    local meta = FindMetaTable("Player")

    function meta:RollDice(min,max,l)

    self.LastRoll = {} -- reset last roll

        if l == nil then l = 1 end -- incase they dont give a third arg

        for i=1, l do 

            self.LastRoll[i] = {res = math.random(min, max), min = min, max = max}

        end

        return self.LastRoll

    end

    concommand.Add("rtd", function(ply,cmd,args)

        if ply.DiceCooldown == nil then ply.DiceCooldown = 0 end -- that should prevent a nil index i believe

        if CurTime() > ply.DiceCooldown + 1 && args[3] <= 5 then -- Cooldown to stop people spamming it just change the number

            net.Start("PlayerRolled")

                net.WriteString(ply:Nick() .. ": " .. PackageTable(ply:RollDice(args[1], args[2], args[3]])))

            net.Send(GetNearbyPlayers(ply:GetPos()))

            ply.DiceCooldown = CurTime()

        else 

            -- tell them there is a cooldown

        end

    end)

    concommand.Add("rtd_last", function(ply,cmd,args)

        if ply.DiceCooldown == nil then ply.DiceCooldown = 0 end -- same as above

        if CurTime() > ply.DiceCooldown + 1 && ply.LastRoll != nil then -- probably an easy way to simplify this to ome command but whatever

            net.Start("PlayerRolled")

                net.WriteString(ply:Nick() .. ": [LAST ROLL]" ..  PackageTable(ply.LastRoll))

            net.Send(GetNearbyPlayers(ply:GetPos()))

            ply.DiceCooldown = CurTime()

        else 

            -- tell them there is a cooldown

        end

    end)

else 

    net.Receive("PlayerRolled", function()

        chat.AddText(net.ReadString()) -- idk how you want it prefixed etc.

        -- iirc last time i used chat.AddText colours just defaulted to bland yellow probs a gamemkde thimg

    end)

end

This should be good to go I *think* added a small list of todos mainly relating to the gamemode functions that I don't have access to. If anyone else notices any problems let me know I'll be happy to change it (or you can I don't mind, just leave credits in).

Finished, live on next crash.

Moved and locked.

Rayts5

Lord Octagon

Spai

Rayts5

Spai

Rayts5

Spai

Voluptious

Brynn