I’m trying to create an advanced attacking system with buffs/debuffs, armors/magic resistance, physical/magical/true damage.
This is the function I ended up with. I find it a bit, ummmh big ? Do you have any feedback on what I could improve/shorten ? And if it isn’t the correct way to go about it, could you explain a better way to do it ?
I guess the final product won’t be short since I want multiple stuff implemented. So yeah I’d appreciate any kind of feedback !
Thx for reading
→ look for post 6 for the small rework of my code and my new question ←
function HitBoxModule:TakeDamage(From, DamageTable) -- From is the guy who is dealing damage, self is the guy receiving damage // When dealing damage a table is passed with 3 different type of damage and their values
if self.Status == "Standby" then -- this is just to activate monsters if they are attacked and were on standby
self.Status = "Chasing"
print("Activating", self.Name, "and chasing", From.Name)
end
if not self.CanTakeDmg then -- for imortality
print(self.Name, "can't receive damage atm")
return
end
local AttackMultiplier = 1 -- this is to get the global attack multiplier from the attacker
table.foreach(From.Multipliers.AT, function(_, v) -- .Multipliers is a table where I keep all the different multipliers for each property (they can come from buff/debuff) // AT is the global attack multiplier
AttackMultiplier *= v
end)
math.max(AttackMultiplier, .2) -- I don't want the multiplier to be smaller than .2
local TotalDamage = 0 -- This is where I'll track the final damage to deal
TotalDamage += (DamageTable.TD ~= nil and DamageTable.TD * AttackMultiplier or 0) -- I'm adding true damage (TD) to the total count, because it ignores armor/magic resist
if DamageTable.AD then -- AD = physical damage
local Multiplier = 1
table.foreach(self.Multipliers.AR, function(_, v) -- AR = armor
Multiplier *= v
end)
local DamageToTake = (DamageTable.AD * AttackMultiplier) - (self.AR * math.max(Multiplier, .2)) -- self.AR represents the armor of the guy being attacked
local ShieldLeft = self.ADShield - DamageToTake -- ADshield is a shield that only stop physical attacks
self.ADShield = math.max(ShieldLeft, 0)
DamageToTake = (ShieldLeft >= 0 and 0 or - ShieldLeft)
TotalDamage += (DamageToTake > 0 and DamageToTake or 0)
end
if DamageTable.AP then -- AP = physical damage
local Multiplier = 1
table.foreach(self.Multipliers.MR, function(_, v) -- MR = magic resist
Multiplier *= v
end)
local DamageToTake = (DamageTable.AP * AttackMultiplier) - (self.MR * math.max(Multiplier, .2)) -- self.MR represents the magic resist of the guy being attacked
local ShieldLeft = self.APShield - DamageToTake -- APshield is a shield that only stop magic attacks
self.APShield = math.max(ShieldLeft, 0)
DamageToTake = (ShieldLeft >= 0 and 0 or - ShieldLeft)
TotalDamage += DamageToTake
end
local ShieldLeft = self.Shield - TotalDamage -- global shield
self.Shield = math.max(ShieldLeft, 0)
TotalDamage = (ShieldLeft >= 0 and 0 or - ShieldLeft)
self.Health -= math.max(TotalDamage, 0) -- dealing the left damage
if self.Health == 0 then -- checking for rewards if he killed the guy
print(From.Name, "killed", self.Name)
end
end
Wow. you are pretty good at both building and scripting. Multi-talent
To answer your question a few things here can be shortened / simplified:
I like your use of short-circuit evaluation but in some cases it impacts readability:
if DamageTable.AD then -- AD = physical damage
local Multiplier = 1
table.foreach(self.Multipliers.AR, function(_, v) -- AR = armor
Multiplier *= v
end)
local DamageToTake = (DamageTable.AD * AttackMultiplier) - (self.AR * math.max(Multiplier, .2)) -- self.AR represents the armor of the guy being attacked
local ShieldLeft = self.ADShield - DamageToTake -- ADshield is a shield that only stop physical attacks
self.ADShield = math.max(ShieldLeft, 0)
DamageToTake = (ShieldLeft >= 0 and 0 or - ShieldLeft)
TotalDamage += (DamageToTake > 0 and DamageToTake or 0)
end
if DamageTable.AP then -- AP = physical damage
local Multiplier = 1
table.foreach(self.Multipliers.MR, function(_, v) -- MR = magic resist
Multiplier *= v
end)
local DamageToTake = (DamageTable.AP * AttackMultiplier) - (self.MR * math.max(Multiplier, .2)) -- self.MR represents the magic resist of the guy being attacked
local ShieldLeft = self.APShield - DamageToTake -- APshield is a shield that only stop magic attacks
self.APShield = math.max(ShieldLeft, 0)
DamageToTake = (ShieldLeft >= 0 and 0 or - ShieldLeft)
TotalDamage += DamageToTake
end
could be reduced to
local Multiplier = 1
table.foreach(self.Multipliers.AR, function(_, v) -- AR = armor
Multiplier *= v
end)
local DamageToTake = (DamageTable.AD * AttackMultiplier) - (self.AR * math.max(Multiplier, .2)) -- self.AR represents the armor of the guy being attacked
local ShieldLeft = self.ADShield - DamageToTake -- ADshield is a shield that only stop physical attacks
self.ADShield = math.max(ShieldLeft, 0)
DamageToTake = (( ShieldLeft >= 0 and 0 ) or - ShieldLeft)
if DamageTable.AP then
TotalDamage += DamageToTake
end
if DamageTable.AD then
TotalDamage += (( DamageToTake > 0 and DamageToTake ) or 0)
end
Seems like quite a lot is going on in that method, so you might wanna just divide some of that code into chunks in separate functions.
This part about magic resist, and I just noticed I could shorten it again a bit. I also think I could regroup the mr/armor calculations in a function to make it simpler for me and avoid repetitions (maybe that’s what you meant with your second advice)
if DamageTable.AP then -- AP = physical damage
local Multiplier = 1
table.foreach(self.Multipliers.MR, function(_, v) -- MR = magic resist
Multiplier *= v
end)
local DamageToTake = (DamageTable.AP * AttackMultiplier) - (self.MR * math.max(Multiplier, .2)) -- self.MR represents the magic resist of the guy being attacked
local ShieldLeft = self.APShield - DamageToTake -- APshield is a shield that only stop magic attacks
self.APShield = math.max(ShieldLeft, 0)
TotalDamage += ((ShieldLeft >= 0 and 0) or - ShieldLeft)
end
After refining my code a bit I have this, I’d like to get the CalculateShield function out of the main one, but since I use the local TotalDamage and ShieldDamageTaken (which are inside the main function) I can’t get it out. Any idea on how to put it outside in a clean way ?
local function GetMultiplier(Table)
local AttackMultiplier = 1
table.foreach(Table, function(_, v)
AttackMultiplier *= v
end)
return math.max(AttackMultiplier, .2)
end
function HitBoxModule:TakeDamage(From, DamageTable)
if self.Status == "Standby" then
self.Status = "Chasing"
print("Activating", self.Name, "and chasing", From.Name)
end
if not self.CanTakeDmg then
print(self.Name, "can't receive damage atm")
return
end
local AttackMultiplier = GetMultiplier(From.Multipliers.AT)
local TotalDamage = ((DamageTable.TD ~= nil and DamageTable.TD * AttackMultiplier) or 0)
local ShieldDamageTaken = 0
local function CalculateShield(Shield, Attack, Armor)
local RawAttack = Shield - math.max(Attack - Armor, 0)
local ShieldAfterAttack = math.max(RawAttack, 0)
ShieldDamageTaken += Shield - ShieldAfterAttack
TotalDamage += (RawAttack >= 0 and 0 or - RawAttack)
return ShieldAfterAttack
end
if DamageTable.AD then
local Multiplier = GetMultiplier(self.Multipliers.AR)
self.ADShield = CalculateShield(self.ADShield, (DamageTable.AD * AttackMultiplier), (self.AR * Multiplier))
end
if DamageTable.AP then
local Multiplier = GetMultiplier(self.Multipliers.MR)
self.APShield = CalculateShield(self.APShield, (DamageTable.AP * AttackMultiplier), (self.MR * Multiplier))
end
self.Shield = CalculateShield(self.Shield, TotalDamage, 0)
self.Health = math.max(self.Health - TotalDamage, 0)
if self.Health == 0 then
print(From.Name, "killed", self.Name)
end
end
Just a few nitpicks to make your code look more shorter
local function GetMultiplier(Table)
local AttackMultiplier = 1
table.foreach(Table, function(_, v)
AttackMultiplier *= v
end)
AttackMultiplier = math.max(AttackMultiplier, .2)
end
local function CalculateShield(arguments) --> CalculateShield(arguments: dictionary)
local RawAttack = arguments.Shield - math.max(arguments.Attack - arguments.Armor, 0)
local ShieldAfterAttack = math.max(RawAttack, 0)
ShieldDamageTaken += arguments.Shield - ShieldAfterAttack
TotalDamage += (RawAttack >= 0 and 0 or - RawAttack)
return ShieldAfterAttack
end
function HitBoxModule:TakeDamage(From, DamageTable)
if self.Status == "Standby" then
self.Status = "Chasing"
print("Activating", self.Name, "and chasing", From.Name)
end
if not self.CanTakeDmg then
return print(self.Name, "can't receive damage atm")
end
local AttackMultiplier = GetMultiplier(From.Multipliers.AT)
local TotalDamage = ((DamageTable.TD ~= nil and DamageTable.TD * AttackMultiplier) or 0)
local ShieldDamageTaken = 0
if DamageTable.AD then
local Multiplier = GetMultiplier(self.Multipliers.AR)
self.ADShield = CalculateShield({Shield = self.ADShield, Attack = (DamageTable.AD * AttackMultiplier), Armor = (self.AR * Multiplier)})
end
if DamageTable.AP then
local Multiplier = GetMultiplier(self.Multipliers.MR)
self.APShield = CalculateShield({Shield = self.APShield, Attack = (DamageTable.AP * AttackMultiplier), Armor = (self.MR * Multiplier)}
end
self.Shield = CalculateShield({Shield = self.Shield, Attack = TotalDamage, Armor = 0}
self.Health = math.max(self.Health - TotalDamage, 0)
if self.Health == 0 then
print(From.Name, "killed", self.Name)
end
end
Is it faster to put the arg in a table ? Or is it just for readability ?
Also, I can’t put the Calculate Shield function outside of the main one because the variables ShieldDamageTaken and TotalDamage are only in the main function (but i’d like to have it outside)
It’s infact slower since indexing a key is not instantaneous. I just tried to prevent having 5 parameters for a function which ig is a case of readability.
Then plug in those variables in the dictionary argument. But you know what I feel like defining the CalculateShield function in the :TakeDamage method is less overcomplicated and neater. Ignore my solution with the arguments dictionary parameter lol.
