Skip to content

Conversation

@ze-dom
Copy link
Contributor

@ze-dom ze-dom commented Jan 2, 2026

🚧 WIP 🚧

To-do:

  • Comment changes with references
  • Update plugin

Developments:

  • Killing Blow multiple hits and magic effect
  • Beast Uppercut multiple hits and magic effect
  • Chain Drive multiple hits and magic effect
  • Dragon Roar multiple hits and magic effect
  • Phoenix Shot multiple hits and magic effect

Bugfixes:

@gemini-code-assist
Copy link

Summary of Changes

Hello @ze-dom, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the combat system by introducing multi-hit capabilities for several Rage Fighter skills. It involves fundamental changes to skill definitions, attack processing logic, and client-server communication to accurately represent and display these multi-hit sequences and their associated new magic effects. The changes aim to complete the implementation of these specific Rage Fighter skills, making them fully functional within the game.

Highlights

  • Multi-Hit Skill Implementation: Introduced a NumberOfHitsPerAttack property to the Skill data model, enabling skills to perform multiple hits per single attack action. This is applied to Rage Fighter skills like Killing Blow, Beast Uppercut, Chain Drive, Dragon Roar, and Phoenix Shot.
  • New Damage Attributes for Rage Fighter Streaks: Added RageFighterStreakHit and RageFighterStreakFinalHit to DamageAttributes to differentiate between non-final and final hits within a multi-hit skill sequence, providing more granular feedback and potential for unique effects.
  • Updated Attack Logic and Network Communication: Modified the core AttackByAsync method across IAttackable implementations (NPCs, Players, SoccerBall) to accept a new isFinalStreakHit parameter. Client-server packets for hit information (SendObjectHitAsync, SendObjectHitExtendedAsync) were updated to transmit these new streak hit flags, allowing the client to display multi-hit effects correctly.
  • New Skill Plugins and Magic Effects: Implemented DragonRoarSkillPlugIn and PhoenixShotSkillPlugIn to handle the area-of-effect and multi-target logic for these Rage Fighter skills. Additionally, new magic effects for 'Decrease Block Effect (Phoenix Shot)' and 'Defense Reduction Effect (Beast Uppercut)' were introduced and initialized.
  • Database Migration: A new database migration (AddSkillNumberOfHitsPerAttack) was added to persist the NumberOfHitsPerAttack property for skills in the database.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces multi-hit skill mechanics for the Rage Fighter class, which is a significant feature addition. The changes span across the data model, game logic, networking, and persistence layers. While the implementation is largely on the right track, there are a few areas that need attention. The logic for handling multi-hit attacks is duplicated in several places, which should be refactored for better maintainability. I've also found a critical bug in the new DragonRoarSkillPlugIn that incorrectly handles single-hit skills. Additionally, there are some minor code quality issues like an inconsistent parameter name and other duplicated logic that should be addressed. Addressing these points will improve the correctness and quality of the code.

if (i <= 3 || Rand.NextRandomBool())
{
// first three 100% chance, others 50% chance
await extraTarget.AttackByAsync(attacker, skillEntry, false, 1, false).ConfigureAwait(false);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There is a bug in how the first hit is handled. The isFinalStreakHit parameter is hardcoded to false. If a skill has only one hit (skill.NumberOfHitsPerAttack == 1), this parameter should be null. This will cause incorrect behavior for single-hit skills that might use this logic. Please use the same pattern as in AreaSkillAttackAction to correctly handle both single and multi-hit skills.

                await extraTarget.AttackByAsync(attacker, skillEntry, false, 1, skill.NumberOfHitsPerAttack > 1 ? false : null).ConfigureAwait(false);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not possible in this context


/// <inheritdoc />
public async ValueTask<HitInfo?> AttackByAsync(IAttacker attacker, SkillEntry? skill, bool isCombo, double damageFactor = 1.0)
public async ValueTask<HitInfo?> AttackByAsync(IAttacker attacker, SkillEntry? skill, bool isCombo, double damageFactor = 1.0, bool? isStreakFinalHit = null)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The parameter isStreakFinalHit should be renamed to isFinalStreakHit to match the method signature in the IAttackable interface. Maintaining consistency with the interface definition is important for readability and prevents confusion.

    public async ValueTask<HitInfo?> AttackByAsync(IAttacker attacker, SkillEntry? skill, bool isCombo, double damageFactor = 1.0, bool? isFinalStreakHit = null)

Comment on lines +2043 to +2051
if (isFinalStreakHit.HasValue)
{
hitInfo.Attributes |= DamageAttributes.RageFighterStreakHit;

if (isFinalStreakHit.Value || this.Attributes[Stats.CurrentHealth] < 1)
{
hitInfo.Attributes |= DamageAttributes.RageFighterStreakFinalHit;
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic for setting the RageFighterStreakHit and RageFighterStreakFinalHit attributes is duplicated from AttackableNpcBase.HitAsync. To improve maintainability and prevent future inconsistencies, this logic should be extracted into a shared helper method. An extension method on HitInfo, such as hitInfo.SetRageFighterStreakAttributes(isFinalStreakHit, isKilled), could be a good approach.

Comment on lines +263 to +269
var hitInfo = await target.AttackByAsync(player, skillEntry, isCombo, 1, skill.NumberOfHitsPerAttack > 1 ? false : null).ConfigureAwait(false);
await target.TryApplyElementalEffectsAsync(player, skillEntry).ConfigureAwait(false);

for (int hit = 2; hit <= skill.NumberOfHitsPerAttack; hit++)
{
await target.AttackByAsync(player, skillEntry, isCombo, 1, hit == skill.NumberOfHitsPerAttack).ConfigureAwait(false);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for performing a multi-hit attack is duplicated in TargetedSkillDefaultPlugin.cs and DragonRoarSkillPlugIn.cs. To improve maintainability and ensure consistent behavior, this logic should be extracted into a single, reusable helper method. For instance, an extension method like IAttackable.AttackWithMultiHitsAsync(...) would be a suitable solution.

Comment on lines +201 to +209
if (isFinalStreakHit.HasValue)
{
hitInfo.Attributes |= DamageAttributes.RageFighterStreakHit;

if (isFinalStreakHit.Value || killed)
{
hitInfo.Attributes |= DamageAttributes.RageFighterStreakFinalHit;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3, MuMain 5.2
Apparently the original didn't check if the target has died, which would cause the damage values not to appear if the target didn't die on the final hit: https://www.youtube.com/watch?v=Vo6aI34Y3FE
But this way we can always show the damage values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3 -> ObjUseSkill.cpp: UseSkill(), SkillAreaMonsterAttack()

/// <summary>
/// Gets the range around the target, in which additional targets are searched.
/// </summary>
protected virtual short Range => 3;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3. This is the range from the target.

public override short Key => 270;

/// <inheritdoc />
protected override short Range => 2;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3. This is the range from the target.

return false;
}

var extraTargets = target.CurrentMap?.GetAttackablesInRange(target.Position, this.Range).Where(FilterTarget).Take(7);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3. We take only 7 since we have already hit 1 earlier and 8 is the maximum for the skill.

{
if (i <= 3 || Rand.NextRandomBool())
{
// first three 100% chance, others 50% chance
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3 -> ObjUseSkill.cpp: UseSkill(), SkillAreaMonsterAttack()

packet.HealthDamage = @healthDamage;
packet.Kind = @kind;
packet.IsRageFighterStreakHit = @isRageFighterStreakHit;
packet.IsRageFighterStreakFinalHit = @isRageFighterStreakFinalHit;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was IsRageFighterStreakHit & IsRageFighterStreakFinalHit added as a custom addition or is it because these new fields hadn't been added to the main version (version 5.2) yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They aren't custom. They were simply missing in the server. As you can see, the client has them.

/// Light pink color, usually used by reflected damage.
/// </summary>
LightPink = 4,
ReflectedLightPink = 4,
Copy link
Contributor Author

@ze-dom ze-dom Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@ze-dom ze-dom Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3 does not have this magic efffect for Phoenix Shot skill, but emu does.
This is a Spanish reference; it states that this effect already existed in S6E2. I found the same thing on Webzen.kr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

powerUpDefinition.Boost = this.Context.CreateNew<PowerUpDefinitionValue>();
powerUpDefinition.Boost.ConstantValue.Value = 16f;
powerUpDefinition.Boost.ConstantValue.AggregateType = AggregateType.AddFinal;
powerUpDefinition.Boost.MaximumValue = 200f;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ SkillNumber.Innovation, MagicEffectNumber.Innovation },
{ SkillNumber.DamageReflection, MagicEffectNumber.Reflection },
{ SkillNumber.BeastUppercut, MagicEffectNumber.DefenseReductionBeastUppercut },
{ SkillNumber.PhoenixShot, MagicEffectNumber.DecreaseBlock },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emu

{ SkillNumber.Weakness, MagicEffectNumber.WeaknessSummoner },
{ SkillNumber.Innovation, MagicEffectNumber.Innovation },
{ SkillNumber.DamageReflection, MagicEffectNumber.Reflection },
{ SkillNumber.BeastUppercut, MagicEffectNumber.DefenseReductionBeastUppercut },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skill.MagicEffectDef.Chance = this.Context.CreateNew<PowerUpDefinitionValue>();
skill.MagicEffectDef.Chance.ConstantValue.Value = 0.4f;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// The decrease block effect of the rage fighter.
/// </summary>
DecreaseBlock = 132,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.CreateSkill(SkillNumber.KillingBlow, "Killing Blow", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Earth);
this.CreateSkill(SkillNumber.BeastUppercut, "Beast Uppercut", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Fire);
this.CreateSkill(SkillNumber.ChainDrive, "Chain Drive", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, abilityConsumption: 20, manaConsumption: 15, levelRequirement: 150, elementalModifier: ElementalType.Ice);
this.CreateSkill(SkillNumber.KillingBlow, "Killing Blow", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Earth, hitsPerAttack: 4); // 1 packet => 1*4 hits
Copy link
Contributor Author

@ze-dom ze-dom Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.CreateSkill(SkillNumber.BeastUppercut, "Beast Uppercut", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Fire);
this.CreateSkill(SkillNumber.ChainDrive, "Chain Drive", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, abilityConsumption: 20, manaConsumption: 15, levelRequirement: 150, elementalModifier: ElementalType.Ice);
this.CreateSkill(SkillNumber.KillingBlow, "Killing Blow", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Earth, hitsPerAttack: 4); // 1 packet => 1*4 hits
this.CreateSkill(SkillNumber.BeastUppercut, "Beast Uppercut", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Fire, hitsPerAttack: 2); // 2 packets => 2*2 hits
Copy link
Contributor Author

@ze-dom ze-dom Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3 (maybe there was only 1 packet previously?), emu. Webzen

this.CreateSkill(SkillNumber.ChainDrive, "Chain Drive", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, abilityConsumption: 20, manaConsumption: 15, levelRequirement: 150, elementalModifier: ElementalType.Ice);
this.CreateSkill(SkillNumber.KillingBlow, "Killing Blow", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Earth, hitsPerAttack: 4); // 1 packet => 1*4 hits
this.CreateSkill(SkillNumber.BeastUppercut, "Beast Uppercut", CharacterClasses.AllFighters, DamageType.Physical, distance: 2, manaConsumption: 9, elementalModifier: ElementalType.Fire, hitsPerAttack: 2); // 2 packets => 2*2 hits
this.CreateSkill(SkillNumber.ChainDrive, "Chain Drive", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, abilityConsumption: 20, manaConsumption: 15, levelRequirement: 150, elementalModifier: ElementalType.Ice, hitsPerAttack: 4); // 2 packets => 2*4 hits
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3 (variable hits 4-7; following this, this might have been an earlier implementation; maybe there was only 1 packet previously?), emu, Webzen

this.CreateSkill(SkillNumber.ChainDrive, "Chain Drive", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, abilityConsumption: 20, manaConsumption: 15, levelRequirement: 150, elementalModifier: ElementalType.Ice, hitsPerAttack: 4); // 2 packets => 2*4 hits
this.CreateSkill(SkillNumber.DarkSide, "Dark Side", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, manaConsumption: 70, levelRequirement: 180, elementalModifier: ElementalType.Wind);
this.CreateSkill(SkillNumber.DragonRoar, "Dragon Roar", CharacterClasses.AllFighters, DamageType.Physical, distance: 3, abilityConsumption: 30, manaConsumption: 50, levelRequirement: 150, elementalModifier: ElementalType.Earth, skillType: SkillType.AreaSkillAutomaticHits);
this.CreateSkill(SkillNumber.DragonRoar, "Dragon Roar", CharacterClasses.AllFighters, DamageType.Physical, distance: 3, abilityConsumption: 30, manaConsumption: 50, levelRequirement: 150, elementalModifier: ElementalType.Earth, skillType: SkillType.AreaSkillExplicitTarget, hitsPerAttack: 4); // 1 packet => 1*4 hits
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.CreateSkill(SkillNumber.IncreaseBlock, "Increase Block", CharacterClasses.AllFighters, DamageType.Physical, distance: 7, abilityConsumption: 10, manaConsumption: 50, levelRequirement: 50, energyRequirement: 80, skillType: SkillType.Buff, skillTarget: SkillTarget.ImplicitParty);
this.CreateSkill(SkillNumber.Charge, "Charge", CharacterClasses.AllFighters, DamageType.Physical, 90, 4, 15, 20);
this.CreateSkill(SkillNumber.PhoenixShot, "Phoenix Shot", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, manaConsumption: 30, elementalModifier: ElementalType.Earth, skillType: SkillType.AreaSkillExplicitTarget);
this.CreateSkill(SkillNumber.PhoenixShot, "Phoenix Shot", CharacterClasses.AllFighters, DamageType.Physical, distance: 4, manaConsumption: 30, elementalModifier: ElementalType.Earth, skillType: SkillType.AreaSkillExplicitTarget, hitsPerAttack: 4); // 1 packet => 1*4 hits
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
result += (int)masterSkillEntry.CalculateValue();
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the TargetAttribute is null we assume the skill adds damage. We need to go all the way back to the ancestor master skills because some may add damage.

attribute.AddElement(new SimpleElement { Value = attrValue });
}

attributeDictionary.Add(targetAttribute, attribute);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need this to make Stats.DefenseDecrement work. Do you see any issues?

result.AttributeCombinations.Add(this.CreateAttributeRelationship(Stats.MaximumHealth, 1, finalBerserkerHealthDecrement, InputOperator.Add, AggregateType.Multiplicate));
result.AttributeCombinations.Add(this.CreateConditionalRelationship(Stats.DefensePvm, statsDefense, finalBerserkerHealthDecrement, AggregateType.AddFinal));
result.AttributeCombinations.Add(this.CreateConditionalRelationship(Stats.DefensePvp, statsDefense, finalBerserkerHealthDecrement, AggregateType.AddFinal));
result.AttributeCombinations.Add(this.CreateConditionalRelationship(Stats.DefenseFinal, statsDefense, finalBerserkerHealthDecrement, AggregateType.AddFinal));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Fire Slash's reduction (now pooled into Stats.DefenseDecrement) is no longer applied before the berserker penalty, we can simplify this.

Comment on lines +897 to +898
this.AddMasterSkillDefinition(SkillNumber.KillingBlowMastery, SkillNumber.KillingBlowStrengthener, SkillNumber.Undefined, 2, 3, SkillNumber.KillingBlowStrengthener, 20, $"{Formula120} / 100", Formula120, Stats.WeaknessPhysDmgDecrement, AggregateType.AddRaw);
this.AddMasterSkillDefinition(SkillNumber.BeastUppercutMastery, SkillNumber.BeastUppercutStrengthener, SkillNumber.Undefined, 2, 3, SkillNumber.BeastUppercutStrengthener, 20, $"{Formula120} / 100", Formula120, Stats.DefenseDecrement, AggregateType.Multiplicate);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zTeamS6.3, emu

The S6E3 client actually has wrong descriptions of these master skills, as they are not the regular skills' magic effects (I've seen this before 😅):

Screenshot from 2026-01-08 16-58-05 Screenshot from 2026-01-08 16-58-22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants