smogon / pokemon-showdown

Pokémon battle simulator.
https://pokemonshowdown.com
MIT License
4.64k stars 2.71k forks source link

Known Mechanics Bugs #2367

Open Marty-D opened 8 years ago

Marty-D commented 8 years ago

Bugs are now listed in the 'Mechanic Bugs' project

https://github.com/Zarel/Pokemon-Showdown/projects/3

You can comment in psim.us/dev or on this bug with any questions or to signal your intent to work on a specific bug listed on the project.

Zarel commented 8 years ago

Mega evolution button fixed in 360ab1f6 and Zarel/Pokemon-Showdown-Client@7f30282084

TheImmortal commented 8 years ago

Team Preview should not show whether Pokemon are shiny or not

Seems as simple as .replace(', shiny', '') in the ruleset, or am I missing something

Zarel commented 8 years ago

Seems as simple as .replace(', shiny', '') in the ruleset, or am I missing something

You'd also need to fix checkDetails to match, and possibly getPokemon.

Zarel commented 8 years ago

In other words, the hard part is making sure the first time the pokemon is switched in, it's recognized as the same pokemon as what appeared in team preview.

Slayer95 commented 8 years ago

@Marty-D , @Zarel : isn't the megaevolution button fix causing a information leak in that its presence will reveal that the user is not move-locked?

Zarel commented 8 years ago

Locked moves aren't information leaks... in-cartridge, it's obvious that a move is locked because it won't ask you what to do at all.

TheImmortal commented 8 years ago

Fixed Parental Bond Secret Power 6603f9703adc6d21da34fc95032d63f6243bc891

ascriptmaster commented 8 years ago

Aiming for actual Parental Bond Secret Power fixing in #2422 since the last fix broke interaction with other abilities

ascriptmaster commented 8 years ago

Parental Bond Secret Power should be fixed now, but I was only able to manual-test for a bit so there might be some edge case that I missed

Zarel commented 8 years ago

Also, when removing checked items, I think they should be commented here (with a link to the fix commit/PR)

Layell commented 8 years ago

Wouldn't changing team preview to icons just make things easier for hiding shinies?

I would also like this for file size and to fit previews for multi-battle in the future.

Layell commented 8 years ago

Also Team Preview shows item/level now so somebody check that off.

https://github.com/Zarel/Pokemon-Showdown-Client/commit/735d9caf1a192 https://github.com/Zarel/Pokemon-Showdown/commit/9b46d7658c5057b824e522e96ed69d4bf17a943f

Zarel commented 8 years ago

The main issue with hiding shinies is the protocol representation, not the visual representation.

Like, it's very straightforward to make the Team Preview sprite not shiny. But the right place to do it isn't there, it's in the protocol.

Marty-D commented 8 years ago

Pressure PP deduction for two-turn moves fixed in https://github.com/Zarel/Pokemon-Showdown/commit/85ddb7dddb8570fa3101c7c643387928d6da2547.

ascriptmaster commented 8 years ago

Normalize fixing in #2449

ascriptmaster commented 8 years ago

2454 fixes the Clear Smog + Anger Point problem.

alex-taxiera commented 8 years ago

I'm going to work on cheek pouch. Could someone explain more of what the suggestion was in the op?

EDIT: I just noticed that cheekpouch only triggers onEatItem but in Fling, Bug Bite and Pluck they do this.singleEvent('Eat', item, null, source, null, null) or this.singleEvent('Eat', item, null, foe, null, null) in the case of Fling.

Changing them to eatItem() seems to work in my tests. Is this just deprecated code that should have been changed or what?

Zarel commented 8 years ago

@alex-taxiera, that would fix it but would be very un-DRY. We generally forgive some level of repetition in the script files, but not that much. The approach we suggest would be a lot better.

1: In-game, it only heals if the eaten Berry has an effect on the user (in the case of Bug Bite/Pluck). Also actually implement Bug Bite/Pluck stealing and eating a Berry activating Cheek Pouch under the above condition. 2: It should only heal after the Berry's effect has happened. Fixing would probably involve renaming EatItem to TryEatItem and having a normal EatItem event after Eat.

For part 2, the event changing involves renaming the event handlers, and changing the event trigger itself in the eat code in the BattlePokemon implementation in battle-engine.js

For part 1, generally investigate how Bug Bite is currently implemented and make sure to either call the same code or copy the EatItem implementation over.

alex-taxiera commented 8 years ago

@Zarel, that's what I was thinking about the repetition. For part one I took a hack at changing the implementation of the moves, which works, but the pokemon apparently also consumes its held item as well so I'll have to check that out more in depth.

For part two it's a problem with the ability being onEatItem so it automatically heals before onEatItem actually does anything, right?

Zarel commented 8 years ago

onEatItem doesn't do the item effect, that happens in the Eat event.

Currently, this is how it works:

  1. EatItem global event happens: anything that happens before the eating, such as Unnerve, happens here.
  2. Eat single-event happens on the item itself: the berry's actual effect takes place here

What we're doing is renaming EatItem to TryEatItem in step 1, and adding a step 3 which will be the new EatItem. All previous onEatItem handlers except Cheek Pouch would be renamed to onTryEatItem.

alex-taxiera commented 8 years ago

Ah, so you're saying the new eatItem should happen after the Eat single-event gets called then. Cheek pouch would then trigger on the new event after the item has gone through Eat

Zarel commented 8 years ago

Yes.

alex-taxiera commented 8 years ago

@Zarel do you think it would work if eatItem called another single-event AfterEat after the Eat event? Then cheek pouch could trigger onAfterEat? I feel like that would fit the existing code well.

Zarel commented 8 years ago

That would also work, yes. The reason I prefer TryEatItem/EatItem to EatItem/AfterEatItem is because that better matches how other events are named.

alex-taxiera commented 8 years ago

Alright, I got it working. I'll just switch the naming over to TryEatItem/EatItem before I submit it then.

alex-taxiera commented 8 years ago

Cheek Pouch fixed in #2497

DanUgore commented 8 years ago

May I just revisit the fact that turn order probably has to be fiddled with anyway to fix the speed tie issue in non-singles. Last I checked that was still a bug, although I don't see it in the list.

Zarel commented 8 years ago

No one's actually explained to me how speed ties are broken in doubles/triples so I have no idea what's even wrong with it...

DanUgore commented 8 years ago

I 100% have. Granted that was a long time ago. Basically resolving speed ties two Pokemon at a time starting with P1's Pokemon gives an edge to P1 in situations where over 2 Pokemon are tied. That's the current problem; Using Array.sort(). I have a partial fix I made a long time ago if you want a reminder on how I implemented it. Although it's pretty dates since my computer it was on died before I pushed time to upload the rest.

Zarel commented 8 years ago

You need to put that in #1157.

Zarel commented 8 years ago

Also that doesn't tell us exactly how the games implement turn order. How do you know the games don't have the same bias?

DanUgore commented 8 years ago

1) The games doesn't have the bias, hence how I know it doesn't have the same bias. If it did it would without a doubt be known. Maybe not super common knowledge but known.

2) How exactly it's implemented I can bring up in #1157

JFernandez-CC commented 8 years ago

I read in the other thread regarding Unnerve that there is an issue with the state of onSwitchIn that is relevant to the issue but that was never explained. What issues do onSwitchIn have with the activation timing of Unnerve?

Zarel commented 8 years ago

onSwitchIn activates separately for each Pokémon.

So if you have Galvantula vs Tyranitar turn 1:

  1. Galvantula switches in
  2. Tyranitar switches in
  3. Galvantula's runSwitch queue entry: onSwitchIn event
  4. Tyranitar's runSwitch queue entry: onSwitchIn event

so if you put Unnerve inside the onSwitchIn event, Tyranitar's Unnerve will run after Galvantula takes SR damage. If you want it to happen before, you need a separate event, and actually a separate queue entry entirely.

18vshanmugam commented 8 years ago

I was testing with transform + mega evolve, it followed all of the rules listed (Charizard transformed into Accelgor with imposter, then mega evolved, and had its mega's stats, sprite, types, but kept Accelgor's moves and had a transform flag). Additionally, this all reset after switching out. Am I missing something, or was this bug already fixed? It is still listed as open on the checklist.

jmclemo6 commented 8 years ago

Hi, I'm looking to get into Programming for PS!. I was wanting to work on the redirection issue. I'm new to this big of a project, but am thinking of making a new event TryFoeRedirectTarget to run before FoeRedirectTarget. Would that be acceptable or does anyone else have any ideas?

alex-taxiera commented 8 years ago

@McLmore I'm not familiar with that bit of code, but that definitely sounds like a good way to start trying to fix the issue.

ascriptmaster commented 8 years ago

@McLmore the event would be named TryRedirectTarget, Foe is a modifier for the event handler function. I'm not sure if it's the best idea to use this kind of event specifically, but if you do, you need to make sure that the two-turn moves handle Power Herb properly, as well as Solarbeam in sun.

TheImmortal commented 7 years ago

The Spread moves and Red Card bug is very relevant to VGC, especially since the Battle Spot ladder in-game won't support it anymore. So PS! is the only place to play VGC '16.

Tagging @Zarel and @Slayer95 for ideas on how to fix.

Marty-D commented 7 years ago

Spread moves and Red Card fixed in https://github.com/Zarel/Pokemon-Showdown/pull/2628.

Marty-D commented 7 years ago

Two-turn moves should ignore redirection on the charging turn fixed in https://github.com/Zarel/Pokemon-Showdown/commit/06567f8d9589e774a639d708086efe994317ea1f.

Marty-D commented 7 years ago

Unnerve should activate before hazards fixed in https://github.com/Zarel/Pokemon-Showdown/commit/bb0b3e064e616bddbdb7fd22e00f0c91097874c7.

Marty-D commented 7 years ago

Interaction between Mega Evolution and Illusion fixed in https://github.com/Zarel/Pokemon-Showdown/pull/2638.

Marty-D commented 7 years ago

Interaction between Primal Reversion and Illusion fixed in #2693.

GraillLord commented 7 years ago

Hi, I apologize for upping this, I'd like to contribute to the pokemon showdown project but I don't know which issues aren't resolved yet. It would be nice if someone could tell me what bugs are still not fixed. (I noticed that http://replay.pokemonshowdown.com/anythinggoes-285817545 has been fixed but is actually still on live, this why I am asking) Thank you in advance.

Marty-D commented 7 years ago

@GraillLord Where do you see that's been fixed? Currently nothing in the checklist is fixed; that's why they're all unchecked and it says 0 of 9 completed on the Issues page.

alchzh commented 7 years ago

Confusion damage should not be affected by opposing Unaware Currently, the activeTarget at the time confusion damage happens is the target of the move that was attempted by the user instead of the user itself, resulting in Unaware affecting the Attack stat stage if that Pokemon was the intended target. Replay Turns 3-4, 7-8

can't we just add this.activeTarget = pokemon;... or does that break something else?

if you yourself have unaware does the confusion take that into account in cart?

@Marty-D

pyuk-bot commented 7 years ago

Currently nothing in the checklist is fixed; that's why they're all unchecked and it says 0 of 9 completed on the Issues page.

Spread moves and Red Card fixed in #2628.

Interaction between Red Card and spread moves Please refer to the Bug Reports post for a brief explanation and replay. Essentially, this is a serious problem with how Doubles and Triples currently handles spread moves: individually hitting each target and running all the relevant effects on the first target before moving to the next one. In-game, each target goes through one step before moving to the next step and going through each target again, much like end-of-turn effects.

@Marty-D

Marty-D commented 7 years ago

@acz13 I don't know, why don't you try it? Unaware doesn't affect confusion damage ever, regardless of which Pokemon has it.

@MacChaeger Whoops, I guess I didn't check it off because the underlying problem still exists but I didn't edit it either.

alchzh commented 7 years ago

@Marty-D is it possible to set activetarget to a null pokemon then?