Skip to content
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

Improve multihit move support #590

Merged
merged 8 commits into from
Feb 26, 2024

Conversation

ShivaD173
Copy link
Contributor

@ShivaD173 ShivaD173 commented Dec 11, 2023

  • Combine TimesUsed and Multihit to use the same between move function previously used by parental bond
  • New/Fixed Effects
    • Stamina (fixed)
    • Weak Armor (fixed)
    • Luminous Moss
    • Kee/Maranga Berry
    • Water Compaction
    • Mummy/Wandering Spirit/Lingering Aroma
    • Seed Sower
    • Sand Spit

@@ -621,18 +596,16 @@ export function calculateSMSSSV(
times
);
const newFinalMod = chainMods(newFinalMods, 41, 131072);
const newBaseDamage = calculateBaseDamageSMSSSV(
const basePower = calculateBasePowerSMSSSV(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is shadowing the basePower defined above.

);
const newBaseDamage = getBaseDamage(attacker.level, basePower, newAttack, newDefense);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this not using calculateBaseDamage?

(defender.hasItem('Luminous Moss') && move.hasType('Water')) ||
(defender.hasItem('Maranga Berry') && move.category === 'Special') ||
(defender.hasItem('Kee Berry') && move.category === 'Physical')) {
const defStat = defender.hasAbility('Kee Berry') ? 'def' : 'spd';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const defStat = defender.hasAbility('Kee Berry') ? 'def' : 'spd';
const defStat = defender.hasItem('Kee Berry') ? 'def' : 'spd';

defender.boosts[defStat] = Math.max(-6, defender.boosts[defStat] - defSimple);
} else {
defender.boosts[defStat] = Math.min(6, defender.boosts[defStat] + defSimple);
if (atkSimple > 1) desc.attackerAbility = attacker.ability;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this condition in the else block? Shouldn't it be in the description regardless if the ability is Contrary or not? Also, it should just be an equality check.

@thejetou thejetou changed the title Combine timesUsed and multihit moves with effects. Improve multihit move support Dec 15, 2023
@ShivaD173 ShivaD173 force-pushed the calc_effects_in_multihit branch from 539f5a7 to 05df464 Compare December 31, 2023 19:11
@thejetou
Copy link
Collaborator

This implementation should also accordingly be in the other mechanics/ files as needed.

Also, can you write unit tests? Multihit code is pretty finicky in the Honko engine so having unit tests would be great for sanity sake.

@ShivaD173
Copy link
Contributor Author

Updated gen3-9 mechanics files for multihit/multiturn accuracy
Modularized code similar to gen789 file to make it easier to do this.
Fixed a few more bugs with displaying abilities, especially with niche items/abilities.
Created tests for multi-hit interactions.

Copy link
Collaborator

@thejetou thejetou left a comment

Choose a reason for hiding this comment

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

I fully read through this diff and grokked it so after these are addressed I'll be able to merge after a quick look through. Sorry for the delay.

field.weather = 'Sand';
}

if (defender.hasAbility('Stamina') && !move.isCrit) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does the move being a critical hit matter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Being a critical hit means that the stamina boosts ignore the defense boosts, and therefore stamina should not be showed in the description. Though now I realize that this doesn't work if they start with negative boosts, Will fix.

@@ -1231,7 +1196,7 @@ export function calculateAttackSMSSSV(
attack = attackSource.rawStats[attackStat];
desc.defenderAbility = defender.ability;
} else {
attack = attackSource.stats[attackStat];
attack = getModifiedStat(attackSource.rawStats[attackStat]!, attackSource.boosts[attackStat]!);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this line change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Original code gets attack stat modified by on screen boost, new code gets attack modified by boost mid calculation, which would include stuff like superpower drops or PuP boosts.

usedItems = checkMultihitBoost(gen, attacker, defender, move,
field, desc, usedItems[0], usedItems[1]);
const newAtk = calculateAttackBWXY(gen, attacker, defender, move, field, desc, isCritical);
const newDef = calculateDefenseBWXY(gen, attacker, defender, move, field, desc, isCritical);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You have to recalculate calculateFinalModsBWXY too so Multiscale works.

// Hack to make Tera Shell with multihit moves, but not over multiple turns
typeEffectiveness = turn2typeEffectiveness;
}
// Stellar damage boost drops off after first hit, even on multihit moves
Copy link
Collaborator

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.

Thanks, I was implementing it how it was on showdown. Will fix.

@ShivaD173
Copy link
Contributor Author

Fixed the critical hit problem.
Fixed the hit input in gen56, and added a test for Multiscale
Changed the Tera stellar interaction with multihit moves.

Copy link
Collaborator

@thejetou thejetou left a comment

Choose a reason for hiding this comment

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

Ready for merge. I'll merge tomorrow though so any unexpected bugs don't happen on SPL Sunday since this is a decently large diff.

@thejetou thejetou merged commit 718d832 into smogon:master Feb 26, 2024
2 checks passed
@thejetou
Copy link
Collaborator

Great work. Thank you!

Celestia74 pushed a commit to Celestia74/eipp-calc that referenced this pull request Oct 17, 2024
Fixes bugs related to Stamina & Weak Armor, and general multihit/multiturn interactions.

Also supports the following abilities/items now too:
- Luminous Moss
- Kee/Maranga Berry
- Water Compaction
- Mummy/Wandering Spirit/Lingering Aroma
- Seed Sower
- Sand Spit
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