-
Notifications
You must be signed in to change notification settings - Fork 98
Outlaw's skills #333
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
Outlaw's skills #333
Conversation
Brick Smash, Throw Sand, Mangle
All skills implemented
All abilities implemented
|
||
hits.Add(skillHit); | ||
|
||
target.StartBuff(BuffId.Common_Silence, skill.Level, 0, TimeSpan.FromMilliseconds(200 * skill.Level), caster); |
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.
In Peltasta's Langort you are using Silence_Debuff, is there a reason to use Common_Silence here?
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.
The skill_bytool for Outlaw specifically names Common_Silence as the debuff. Langort might be incorrect, as the comment indicates the buff is not in the bytool, so I may have chosen the wrong Silence debuff, though multiple buffs with the same effect in this game is not uncommon.
/// <summary> | ||
/// Increases the buff's duration by a given amount. | ||
/// </summary> | ||
internal void IncreaseDuration(TimeSpan amount) |
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.
Probably better to merge IncreaseDuration()
and ExtendDuration()
into a single method.
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.
I agree this is somewhat confusing and maybe the description should have been a little more clear, but ExtendDuration has several features we don't want here, most particularly that it resets the updatetime on the buff. We probably don't want to constantly call onExtend here either.
/// </summary> | ||
/// <remarks> | ||
/// NumArg1: Skill Level | ||
/// NumArg2: Extra 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.
Am I right in assuming that num arg 2 is a copy/paste error? It doesn't seem to get used, and the value passed is 0.
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.
Yeah, that's a mistake.
target.StartBuff(BuffId.Aggress_Debuff, skill.Level, unhingeLevel, duration, caster); | ||
target.StartBuff(BuffId.ProvocationImmunity_Debuff, skill.Level, 0, duration, caster); | ||
|
||
if (target.Components.TryGet<AiComponent>(out var component)) |
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.
I guess we might want to think about some way to make this simpler, so we don't always have to query the component, or even think about what goes into a hate alert.
|
||
hits.Add(skillHit); | ||
|
||
if (RandomProvider.Get().Next(10) <= stunChance) |
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.
While not a big issue, you might want to consider saving the RNG if you need to use it repeatedly in a loop. Saves a tiny bit of time and feels more proper.
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.
Is that safe to do? No potential issues if other processes call RNG in the meantime?
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.
No, the whole purpose of RandomProvider
is to create a thread-safe Random
instance.
// Splash area is a square of length 300, width 100, starting 100 | ||
// units behind the caster | ||
var splashArea = new Square(caster.Position.GetRelative(caster.Direction, -100f), caster.Direction, 200f, 100f); | ||
Debug.ShowShape(caster.Map, splashArea); |
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.
I'm guessing this was left here by accident? We don't usually leave live debug code in the main repo.
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.
Oops, yes, please remove that.
Adds all 7 skills for outlaw and all of their abilities, including the removed ones.