smogon / damage-calc

Pokemon games damage calculator
https://calc.pokemonshowdown.com
MIT License
369 stars 345 forks source link

Improve Stellar handling #605

Closed Zrp200 closed 3 months ago

Zrp200 commented 3 months ago

Currently, Tera Stellar is shown in too many cases. When it's active, it is always shown, but this is misleading:

Even in the case of Tera Blast, the first use has a different stab modifier, but both cases just display 'Tera Stellar' and 100 BP, which is incorrect --- the same description should ideally always yield the same numbers.

Zrp200 commented 3 months ago

The only hitch is that you still can't specify timesUsed in the calculator for Tera Blast Stellar.

https://github.com/smogon/damage-calc/blob/ecc38a87048580a2a97b253ccb582246c4d614b8/src/js/shared_controls.js#L509 This doesn't apply in this case, since dropsStats is only set after the calculation starts. A workaround solution is possible, but https://github.com/smogon/damage-calc/blob/ecc38a87048580a2a97b253ccb582246c4d614b8/calc/src/move.ts#L134 prevents actual passing of the parameter to begin with, haha. Still looking into this, however.

thejetou commented 3 months ago

The only hitch is that you still can't specify timesUsed in the calculator for Tera Blast Stellar.

https://github.com/smogon/damage-calc/blob/ecc38a87048580a2a97b253ccb582246c4d614b8/src/js/shared_controls.js#L509

This doesn't apply in this case, since dropsStats is only set after the calculation starts. A workaround solution is possible, but https://github.com/smogon/damage-calc/blob/ecc38a87048580a2a97b253ccb582246c4d614b8/calc/src/move.ts#L134

prevents actual passing of the parameter to begin with, haha. Still looking into this, however.

Hm, I think a good solution would be to set an overrides of self.boosts to Move if the move is Tera Blast Stellar in shared_controls.js.

Zrp200 commented 3 months ago

Whoops. My ide was complaining about var declarations, I told it to shut up, and it put that there. I thought I avoided committing that, but there it is. I'll remove it when I get back to my computer.

On Sat, Mar 16, 2024, 12:14 PM Waleed Hassan @.***> wrote:

@.**** commented on this pull request.

In src/js/shared_controls.js https://github.com/smogon/damage-calc/pull/605#discussion_r1527220535:

@@ -1,3 +1,5 @@ +// noinspection ES6ConvertVarToLetConst

?

— Reply to this email directly, view it on GitHub https://github.com/smogon/damage-calc/pull/605#pullrequestreview-1941296862, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWABOUIS6ALK3SUHQ3M6YLYYSKX7AVCNFSM6AAAAABEYZYAQ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNBRGI4TMOBWGI . You are receiving this because you authored the thread.Message ID: @.***>

Zrp200 commented 3 months ago

The only hitch is that you still can't specify timesUsed in the calculator for Tera Blast Stellar. https://github.com/smogon/damage-calc/blob/ecc38a87048580a2a97b253ccb582246c4d614b8/src/js/shared_controls.js#L509

This doesn't apply in this case, since dropsStats is only set after the calculation starts. A workaround solution is possible, but https://github.com/smogon/damage-calc/blob/ecc38a87048580a2a97b253ccb582246c4d614b8/calc/src/move.ts#L134

prevents actual passing of the parameter to begin with, haha. Still looking into this, however.

Hm, I think a good solution would be to set an overrides of self.boosts to Move if the move is Tera Blast Stellar in shared_controls.js.

Is this not what the current implementation does? https://github.com/smogon/damage-calc/blob/a5551abe9b828d8fe0c5a857d62c3de27a7e8eca/src/js/shared_controls.js#L1068

thejetou commented 3 months ago

Thank you!