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

TODO List, task No.5 #200

Merged
merged 5 commits into from
Dec 30, 2017
Merged

TODO List, task No.5 #200

merged 5 commits into from
Dec 30, 2017

Conversation

Ntonio36
Copy link
Contributor

@Ntonio36 Ntonio36 commented Dec 30, 2017

MK360 hits again
Basically shows KO chances when using a stat-dropping move several turns in a row, with some (big) cleanups.
Commits :
index.template.html : add dropdown for showing how many times this select move is used in a row.
damage.js : refactor + calculate stat-drop induced damage. Also handles Contrary and White Herb.
damage_rby.js : adding description in case a Pokémon used such a move.
item_data.js : refactor Multi-Attack item type detection. Somehow, no one noticed the type of the Memory held is fully mentioned in its name.
move_data.js : added a property for moves that drop stats, whose value is equal with by how much does a stat drop.
shared_controls.js : added the ability to handle - guess what - moves that drop stats by showing their select dropdown, and also binding a property that is used in several places.
ko_chance.js : little refactor + built a workaround to deal with multi-turn moves.

js/damage.js Outdated
@@ -298,7 +302,6 @@ function getDamageResult(attacker, defender, move, field) {
break;
case "Nature Power":
basePower = (field.terrain === "Electric" || field.terrain === "Grassy" || field.terrain === "Psychic") ? 90 : (field.terrain === "Misty") ? 95 : 80;
//console.log("A " + field.terrain + " terrain " + move.type + move.name + " with " + move.bp + " base power " + " agaisnt a(n) " + defender.name + " that has " + defender.type1 + " " + defender.type2 + " typing");
Copy link
Member

Choose a reason for hiding this comment

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

I might actually still want this, I was using to be sure terrain and nature power were working correctly

js/damage.js Outdated
for (var times = 0; times < move.usedTimes; times++){
var oldAttack = attack;
var newAttack = getModifiedStat(attacker.rawStats[attackStat], attacker.boosts[attackStat]);
damage = damage.map(function(affectedAmount) {
Copy link
Member

Choose a reason for hiding this comment

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

Missing parentheses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uuuuuuh where are they missing ? Neither Sublime Text nor GitHub gave me anything that pointed at a mistake here, even though I could be wrong..

Copy link
Member

@AustinXII AustinXII Dec 30, 2017

Choose a reason for hiding this comment

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

Yeah I was actually just commenting never mind as you typed that lol. It looked wrong on mobile at first mb

return 'Water';
default:
return '';
if (item.indexOf("Memory") !== -1) {
Copy link
Member

Choose a reason for hiding this comment

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

Plates works the same way, do they not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Plates' names are like "Meadow Plate", "Flame Plate", while Memories directly point at type such as "Fire Memory", "Grass Memory", etc. so instead of looping through every case we just cut "Fire" from "Fire Memory" and we return it.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I forgot the plate naming was different :p

return 'possible ' + i + 'HKO' + afterText;
} else {
qualifier = 'nearly ';
// until someone comes up with a better damage recalculation formula, we'll tell the user it's an estimate
Copy link
Member

Choose a reason for hiding this comment

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

I don’t know if this is necessary..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned, the multi-turn moves formula is a bit brutal, but couldn't find a more accurate way to deal with the new attack value.

@AustinXII
Copy link
Member

Besides that, looks good!

js/damage.js Outdated
@@ -545,8 +548,7 @@ function getDamageResult(attacker, defender, move, field) {
description.defenderAbility = defAbility;
}

if ((gen < 7 && defender.item === "Soul Dew" && (defender.name === "Latios" || defender.name === "Latias") && !hitsPhysical) ||
(defender.item === "Assault Vest" && !hitsPhysical) || defender.item === "Eviolite") {
if (gen < 7 && (!hitsPhysical && ["Latios","Latias"].indexOf(defender.name) !== -1) || defender.item === "Eviolite" || (!hitsPhysical && defender.item === "Assault Vest")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing check for Soul Dew

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad ^^".

@Ntonio36
Copy link
Contributor Author

Demo time!

On selecting any move (here, Draco Meteor), this dropdown shows up :
dropdown
Now because we're at "Once", the calculator behaves like it's a traditional single-hit move :

one hit

But when we raise the usage times.....

5 hits

This image shows how much did our Abomasnow hurt the other when raining Meteors for 5 times in a row.
Now this case doesn't include White Herb. Here is our item in action :
3 hits
In the following image we used Draco Meteor 3 times, so its SpA should've dropped to -6. But White Herb interfered with the first drop action and nullified it, leaving 2 drops left (hence the -4). And since the main goal is showing us whether this usage KOes the opponent or not, well... it doesn't.

@AustinXII
Copy link
Member

Great work!

@AustinXII AustinXII merged commit 2ec87f8 into smogon:master Dec 30, 2017
@Ntonio36 Ntonio36 deleted the patch-1 branch December 30, 2017 18:11
@penpexgit
Copy link
Contributor

I don't know if I should comment here or on the following fixes but there are problems with this modification:

  1. I did a few tries with Draco Meteor on the two default Abomasnow (for the sake of example):
    So the first hit does minimum 183 damage HP. Then a second hit would be at -2 SpA, and do minimum 92 damage HP (still using the 'Once' option, which should reflect how the calc worked before this update). So the minimum damage of using the new option with 'Twice' should show a minimum of 183+92=275 damage HP, but it shows a minimum of 274 damage HP. Now I know that there are subtleties with the way the rounding off is done, but since in this case it's supposed to be two moves executed after another, if my logic is sound it should really be a minimum of 275 damage HP.
    I really think this is not a rounding off error as the following will show:
    If you continue, and do a third hit, then it should be at -4 SpA and add (still using the 'Once' option) another minimum of 61 damage HP, and therefore the minimum damage should be a minimum of 183+92+61=336 damage HP, but using the '3 times' option shows a minimum of 364 damage HP, that's not a small difference!
    It gets even worse:
    Adding a fourth hit, which should be at -6 SpA, should add a minimum of 45 damage HP, so using the '4 times' option we should get a minimum of 183+92+61+45=381 damage HP, but we get 454!
    Finally, adding a fifth move, still at - 6 SpA then, should add again another minimum of 45 damage HP, and so using the '5 times' option should show a minimum of 183+92+61+45+45=426 damage HP, but we get a minimum of 566 damage HP!
    The amount of minimum damage added between '3 times' -> '4 times' and '4 times' -> '5 times' which should be the same is not even the same.
    So there is clearly something really wrong going on here, and I didn't test other moves but it is probably the same then.

  2. In the 'demo' case shown above, when Draco Meteor hits 3 times, do we really want to show the SpA at -6 in the result text?
    I feel like that we should show either:

  • Just the number of times the moves was used, which is already implemented and shown
  • Show -4 because the third drop happens after the hit. It seems misleading to show -6 in my opinion, since it is not the default showing for all the other moves that show what stat is used to calculate the damage and therefore before the move is used. Especially since if you start at let's say -1 SpA, the calc will show -5 SpA after the 'twice' option which is really confusing because it mixes up at stat before use and a stat after use.
  • Show the starting stat modifier and the number of times the move was used, which is probably the option that has my preference, seeing what I said above. That would make multiple attacks look like all other moves, and seemingly counted as one entity.
    I think this should at least be discussed, and in that case it's just the text that would have to be changed.

@AustinXII
Copy link
Member

@Ntonio36 please fix or I’m reverting this, this has caused a lot of issues 0:

@penpexgit penpexgit mentioned this pull request Jan 5, 2018
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.

4 participants