MY first real work tasked by a friend

[UndeadSold1er]

AddCSLuaFile("shared.lua")
include('shared.lua')

function ENT:SpawnFunction(ply, tr)
	if !tr.Hit then return end
	
	local SpawnPos = tr.HitPos + tr.HitNormal * 1
	
	local ent = ents.Create("boxOfHealth")
	ent:SetPos(SpawnPos)
	ent:Spawn()
	ent:Activate()
	mathVar=0
	
	return ent
end

function ENT:Initialize()

	self.Entity:SetModel("models/props_lab/citizenradio.mdl")
	
	self.Entity:PhysicsInit(SOLID_VPHYSICS)
	self.Entity:SetMoveType(MOVETYPE_VPHYSICS)
	self.Entity:SetSolid(SOLID_VPHYSICS)
	
	self:SetUseType(SIMPLE_USE)
	
	self.Index = self.Entity:EntIndex()
	
	local phys = self.Entity:GetPhysicsObject()
	if phys:IsValid() then
		phys:Wake()
	end
end


function ENT:Use(activator)
	if activator then
		mathVar=mathVar+1
	end
			
	Msg(mathVar.."\n")
	
	if mathVar == 3 then
		activator:SetHealth(activator:Health()+5)
		self.Entity:Ignite(5)
		timer.Simple(5, function() self.Entity:Remove() end)
		//self.Entity:Remove()
		mathVar=mathVar-3
		self.Entity:SetNotSolid(true)
	end
end

started with a simple eat script then my friend asked me to specifically make it catch on fire for 5 seconds then go poof after pressing E "3" times, I am learning Lua so this was difficult and most likely my ways to make timings etc. was probably barbaric so any suggestions to make it look neater would be welcome and the bottom function and middle functions are the only ones with any sort of noticeable change. THANKS!

[editline]5th October 2015[/editline]

ohh and ignore //self.Entity:Remove() I forgot to remove it from the block

[MrSpore]

So, what exactly is the objective of the post?

[UndeadSold1er]

[QUOTE=MrSpore;48836829]So, what exactly is the objective of the post?[/QUOTE]
Suggestions on what to improve/ not do next time

[Netheous]

Well, you didn't really make a lot of code so it's hard to tell you what you should improve on.

[MuteTM]

You have mathVar as a global.

[Whiterabbit]

1. All your references to [I]self.Entity[/I] could (and should) just be [I]self[/I] since you are already coding an entity.

2. As MuteTM said, mathVar is a global. Try spawning 1 entity, pressing E on it twice, then spawn a new entity. mathVar will be reset to 0 and the first entity will require another three presses. Same issue if you spawn two entities, press E on the first one twice, the press E on the second one. The first press on the second entity will count as it's third press.

3. Inside your timer you should check if the entity is still valid. You have a 5 second timer; let's say you trigger the timer, but before 5 seconds is expired you undo the entity. Now when the timer fires the entity no longer exists and you get an error. Fix:

timer.Simple(5, function() self.Entity:Remove() end)

--Change it to

timer.Simple(5, function()
    if not IsValid(self) then return end
    self:Remove()
end)

[MysteryPancake]

:snip:

[jaooe]

Maybe use more descriptive variable names, it's not clear what mathVar is supposed to be.

[Sestze]

I imagine it'd be better if you put the number of uses inside of the entity itself.

ent.Uses = 0 instead of mathVar

[DevulTj]

[QUOTE=Sestze;48852405]I imagine it'd be better if you put the number of uses inside of the entity itself.

ent.Uses = 0 instead of mathVar[/QUOTE]

You might want to explain to him the benefits of this.

Basically, if you have a global variable designed for use of 1 entity, it takes the factor of any other entity having the same system running. It would not be fun to use. Let me go into some detail.

If you had two of those entities and used one, it would have the same variable on the other. You would have to put ent.mathVar instead of mathVar if you would like the variable to be local to the entity itself - that means that the variable belongs to 1 certain entity on the map so you will have no issue with using multiple entities on the map.

[B]Edit: [/B] Just noticed that wh1t3rabbit has explained it already.

[UndeadSold1er]

[QUOTE=mib999;48853445]You might want to explain to him the benefits of this.

Basically, if you have a global variable designed for use of 1 entity, it takes the factor of any other entity having the same system running. It would not be fun to use. Let me go into some detail.

If you had two of those entities and used one, it would have the same variable on the other. You would have to put ent.mathVar instead of mathVar if you would like the variable to be local to the entity itself - that means that the variable belongs to 1 certain entity on the map so you will have no issue with using multiple entities on the map.

[B]Edit: [/B] Just noticed that wh1t3rabbit has explained it already.[/QUOTE]

Thanks guys you actually pointed out the problems I have expirienced and fixed in awkward ways but you made it simpler to fix and without those wierd ole fixes.