pokerogue
pokerogue copied to clipboard
pagefaultgames
Critical Hits, Super effective, multi-hit moves dialogue
Fixes issue #680
---- original behavior on main, on faint doesn't display effectiveness dialogue, or multi hit result:
https://github.com/pagefaultgames/pokerogue/assets/39450497/919527d3-7a3a-4ea4-8c84-7f17b746be67
---- PR fix: critical hits, effectiveness, and multi-hit results (even if ended early due to faint) how properly show when fainting
https://github.com/pagefaultgames/pokerogue/assets/39450497/f392e989-6380-4e34-9b4f-2d68631979eb
---- some more bugs to do?
For moves like absorb that have subsequent dialogue, it runs into the same issue of not rendering on faint, but it was not fixed with this PR, perhaps this can be done in a separate issue, needs more investigating. More generally the end() function of the MoveEffectPhase class in phases.ts doesn't queue message properly on faint, it seems like those messages are lose
https://github.com/pagefaultgames/pokerogue/assets/39450497/0d346fdd-dc1a-40a4-8a67-5fa3563340b4
Believe I have found the fixes to my "more Bugs to do", will update the PR shortly
Found a cleaner solution to the multi-hit dialogue, as well as resolving the problems referenced above in ---- some more bugs to do?
new current display:
https://github.com/pagefaultgames/pokerogue/assets/39450497/e039d054-395a-41bc-80ac-ea9efb7a7dce
https://github.com/pagefaultgames/pokerogue/assets/39450497/ac513dc7-121f-4d12-ac95-cc2002ea1723
seems like the conflicts are easy to resolve: simply take the incoming change, notice that the if/switch statement has already been defined above by me
for future reference, who deals with merge conflicts, the reviewers or the PR author?
I pulled and tested this with various of things.
I hope that it's intended that it does it like this if the Pokemon dies
--> Tells u it's not very effective (depends what u did) --> Pokemon I attacked Faints with text --> Tells me my EP --> If I level up it shows that --> Teaching attacks come also here --> Tells you it has been hit X times (at the end of all of it 🤷, but it works!)
One of the first thing I did was to use the MULTI_LENS.
And it works.
If I use a multi attack, it will also stop once fainted and tell me the count, e.g. "hit 1 time"
For every hit, if it's a critical one, it will tell me as well, right after the hit.
Then I tried a double battle
I tried to defeat the pokemon at the right to see what happens.
After it gets fainted it will tell you the hit count as well
The next thing I tried, was an attack that can hit multiple Pokemon + the Multi Lense thing in a double battle
I chose Earthquake.
And I encountered an issue, but I think this is because that's how double battles work right now.
Result was like so
--> Charmander fainted --> The enemy at the left fainted --> I get 23 EP --> The enemy at the right fainted --> I get 27 EP --> I hit 3 times
So that works. It shows the hit result. I can't test K.O. messages because I am unfamiliar with the attacks. Maybe Explosion is one or something.
If one of the Pokemons in a double battle faints. It will not tell me the hit result, until my attack actually finished. All other messages, including level ups show up.
Now, I am not sure, but if Level Ups happen before the hit result, would it also mean, that the Pokemon can learn new attacks and additionally evolve? Not sure, idk. I didn't read the source code much.
Then I tried it with a Pokemon that can Float in the air
Earthquake will also still continue until it all the attack streaks are finished and it will tell me the hit count. Well, it never hit the floating pokemon on my team after the other pokemons in the double battle were defeated and it continued the Earthquake attack.
But the hit count was correct, the attack happened 3 times.
Now, I read up the other things.
I will write shortly, but I just noticed it never told me, that Earthquake has no effect on Flying Pokemon.
I encountered this in a Double Battle.
thanks for such a extensive write up karl, the intention is to show the number if hits before the fainting. I'll look into reproducing your set-up. In the original PR post i've included it properly displays the info in that order, I wonder what happened...
Now I tried, Leftovers and Multi-Lense
With the opponent having spore and rough skin and I gave myself leftovers.
Idk wtf I did
Game froze here, could be a different bug
See in the console
It ended the TurnPhase, then it started another one 🤷
Oh I think I get it maybe.
@DustinLin So, the opponent Pokemon in the double battle had Rough Skin. I am not sure if the game froze, because my Pokemon died, but the attack still wanted to run.
The attack I used was Ice Fang with Multi-Lense.
The thing is, it did tell me how many times I hit the Pokemon, once it told me that my Pokemon flinched, the game froze.
Could be a different bug 🤷
that seems like a different bug, if a pokemon dies it gets it's BattleScene removed, if that same pokemon was queued to do something, then referencing it's null scene could lead to the behavior you just got
Spore never triggered, so I removed Rough Skin now, and now I will test that. Maybe Spore is not a passive, idk.
Actually I got rid of Spore and just made it Species.AMOONGUSS and put spore on passive and ability
started a new run
used fury swipes spore triggered
and it continued, but I think this PR is not about fixing that, so it shouldn't matter
So the only thing that I am unsure about, is the Flying Pokemon in the Double Battle not showing that the attack had no effect 🤷
That could be a multi-lense issue. It's not only in double battles.
I removed multi-lense and it did still not show me the text though, that Earthquake doesn't work on the Flying Pokemon. @DustinLin
Also removed Leftovers and same thing.
Then I tried self-destruct
The K.O message never showed up. (That's what I was testing)
And my game froze and I managed to make it to the next wave
@DustinLin
Could be a different issue. Thing is, K.O. Message and the message that tells me that Earthquake has no effect, are not showing up.
thanks for such a extensive write up karl, the intention is to show the number if hits before the fainting. I'll look into reproducing your set-up. In the original PR post i've included it properly displays the info in that order, I wonder what happened...
I don't know. I am using Github Desktop. And clicked on the pull and then it did the rest.
I hope it was right and up-to-date 🤔
@karl-police i figured out the bug behind the no-effect behavior, it has to do with conditioning the messages on if the move actually does damage or not (~line 1645 in pokemon.ts), I should probably include the fix in this commit, but it seems like the very same code is refactored in PR #924 (and seems to work properly in that case) so I'm not sure what to do.
The self-destruct is definitely something that needs to be fixed so I'll look into that, thanks!
@karl-police i figured out the bug behind the no-effect behavior, it has to do with conditioning the messages on if the move actually does damage or not (~line 1645 in
pokemon.ts), I should probably include the fix in this commit, but it seems like the very same code is refactored in PR #924 (and seems to work properly in that case) so I'm not sure what to do.The self-destruct is definitely something that needs to be fixed so I'll look into that, thanks!
I'd ask @bennybroseph
If he replies.
Thanks a lot for all the work you're doing! This has been bugging me since I started playing (even though it doesn't prevent me from enjoying the game!).
While testing the changes proposed in the PR (at this commit), I found a new bug, which is the pokemon doesn't faint anymore if he dies from a status effect. I believe we might have some other new ones, since #damageAndUpdate is used quite a lot in different places.
Video:
https://github.com/pagefaultgames/pokerogue/assets/57403591/036c6a2a-c2c5-4807-966d-e1ebe6620ce8
I'm proposing in this PR a way to resolve the order of messages only. I'm not taking care of the multi-hit moves (I believe it's still something we want, but we can maybe have it in separate PRs if that makes sense).
- https://github.com/pagefaultgames/pokerogue/pull/1182
@karl-police I believe I have fixed the game breaking self-faint bug you pointed out. I've tested and things still seem to be in order, it would be great to get some double confirmation.
The multi-hit dialogue is a larger issue, I'm proposing that for this PR (so that we can get it in and subsequent things merged and settled) we just include these changes. It could be the case that subsequent PRs solve it, but I'm doubtful.
The issue is essentially that the multi-hit message phase is being added too late, and other combat resolving code that is being run is messing with the order in which Phases are being added to the PhaseQueue. (more details are in the comments of the code of the commit) I have a few ideas for how to fix it, but it will take me a bit more time to figure out everything that is going on there before I start messing around with that code, so I want to leave that for another issue/PR.
Long thread going on, which is good for progress. Hard to process though. Could we get a recap of what bugs still exist in this PR if any and what is still being worked on?
@karl-police I believe I have fixed the game breaking self-faint bug you pointed out. I've tested and things still seem to be in order, it would be great to get some double confirmation.
The multi-hit dialogue is a larger issue, I'm proposing that for this PR (so that we can get it in and subsequent things merged and settled) we just include these changes. It could be the case that subsequent PRs solve it, but I'm doubtful.
The issue is essentially that the multi-hit message phase is being added too late, and other combat resolving code that is being run is messing with the order in which Phases are being added to the PhaseQueue. (more details are in the comments of the code of the commit) I have a few ideas for how to fix it, but it will take me a bit more time to figure out everything that is going on there before I start messing around with that code, so I want to leave that for another issue/PR.
I think the message appearing too late is not as bad as not appearing at all.
Immunity message thingy works now
However, it says that it doesn't effect the Steel Pokemon over there. But that's only because it's floating. Shouldn't it have a different message?
Self-Destruct works too, but the K.O. messages don't appear? Not sure if it's ment to have one.
Then I tested Self-Destruct with the Recovery Seed. It didn't heal me back though when I died. But Recovery Seed worked without Self-Destruct usage.
Death by Poison seems to work too, I think. I tried it with Poison Point.
I think that is the intended dialogue for levitate? It seems like the ability is showing, and i'm guessing the dialogue reads "it has no effect".
As to @bennybroseph 's message to recap: there were a couple problems karl-police found when testing
- The multi hit dialogue is still out of order - I've found the root cause of this issue but it may take a bit to fix. I think the best plan is to include it in another PR (as karl-police mentioned it's better than not appearing at all at the moment).
- There seems to be a couple bugs regarding the items Multi-lens and recovery seed? (Paraphrasing karl-police here) This seems to be a separate issue and not related to the changes here though.
I think that is the intended dialogue for levitate? It seems like the ability is showing, and i'm guessing the dialogue reads "it has no effect".
As to @bennybroseph 's message to recap: there were a couple problems karl-police found when testing
- The multi hit dialogue is still out of order - I've found the root cause of this issue but it may take a bit to fix. I think the best plan is to include it in another PR (as karl-police mentioned it's better than not appearing at all at the moment).
- There seems to be a couple bugs regarding the items Multi-lens and recovery seed? (Paraphrasing karl-police here) This seems to be a separate issue and not related to the changes here though.
When I looked at Phaser and these "Phases", my first thought was How do actual Pokemon games code it. Because it all seems very very dank that everything is encapsulated in these Phases.
I think aslong there's no other bugs, it should be fine. Recovery Seed works, when I use Self-Destruct it doesn't revive me, that could be a different issue, maybe even intended.
Thanks for reminding me, but I was the one who posted a writeup explaining Phases and how they work in the discord a couple weeks ago, I can maybe add that documentation in another PR, or wherever @bennybroseph thinks would be a good fit for it
Where's that K.O. message though? I think that's the only remaining mystery. battle:hitResultOneHitKO
I think that may be for 1-hit KO moves? Like fissure. Explosion/self-destruct are not one-hit KO moves so i don't think they classify. I haven't played any of the newer main series games, but I don't think a KO dialogue appears in those?
Hey all, I've merged in the latest changes from main, which includes PR #1182 from @francktrouillez . many of our changes are similar, so I fixed the conflicts and added/removed from both of ours. Let me know if there are any questions, but basically the cases are:
- multi hit move ending early due to faint, still want to show effective-dialogue
- some moves (like absorb) have additional dialogue that needs to be displayed before faint
These mostly have to do with the setting of phaseQueueSpliceIndex
I believe I also fixed the multi-hit dialogue order, I've tested all of @karl-police 's cases and they seem to work out fine? If any of you 2 could also confirm this that would be great.
https://github.com/pagefaultgames/pokerogue/assets/39450497/2ad23898-5825-45ef-8207-74b28d47fd33