-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding the rest of Assassin's Skills and Abilities #334
base: master
Are you sure you want to change the base?
Conversation
All skills and abilities implemented.
@@ -647,7 +647,7 @@ public float SCR_GetCritChance(ICombatEntity attacker, ICombatEntity target, Ski | |||
return 100; | |||
|
|||
var critDodgeRate = target.Properties.GetFloat(PropertyName.CRTDR); | |||
var critHitRate = attacker.Properties.GetFloat(PropertyName.CRTHR) + modifier.BonusCritChance; | |||
var critHitRate = attacker.Properties.GetFloat(PropertyName.CRTHR) * modifier.CritRateMultiplier + modifier.BonusCritChance; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick note here but multiplying the CRTHR by 2 (for Annihilation ability bonus) is not the same as multiplying the critChance variable by 2 due to the exponential formula it has.
Example:
critHitRate = 200
critDodgeRate = 50
No Annihilation Buff:
(200 - 50) ^ 0.6 = 20.2% crit chance
Annihilation Buff (2x Crit Chance):
(400 - 50) ^ 0.6 = 33.6% crit chance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct, this probably also applies to BonusCritChance as well.
modifier.HitCount = 10; | ||
|
||
// Assassin15 adds double crit rate to bleeding targets | ||
if (caster.IsAbilityActive(AbilityId.Assassin15) && (target.IsBuffActive(BuffId.HeavyBleeding) || target.IsBuffActive(BuffId.Behead_Debuff))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assassin15 is the Annihilation enhance damage ability. Don't you mean Assassin17?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, it is indeed Assassin17.
{ | ||
/// <summary> | ||
/// Handler for the ThrouwingSpear_Hoplite33_Pad, | ||
/// which displays an explosion effect in a given position |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor description error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Based on Kenedos' comments
[SkillHandler(SkillId.Assassin_HallucinationSmoke)] | ||
public class Assassin_HallucinationSmoke : IGroundSkillHandler | ||
{ | ||
private readonly static TimeSpan DebuffDuration = TimeSpan.FromSeconds(5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not being used
All skills and abilities implemented, including the removed abilities.
Note that Common_Silence is implemented in Outlaw, so I haven't included it here.