Confusion self-hit doesn't trigger Sitrus + super sitrus Berries

[QuiteQuiet]

Mentioned in #2367 (comment)

Not sure if this is the best way to implement this, but I can't think of anything else.

@Marty or @DaWoblefet can either of you confirm if Oran Berry/Berry Juice/Stat-boosting berries also does this or if it's only the healing berries. Also is this new in Gen 7, or was this also the case before?

[DaWoblefet]

I can confirm this has been the case since Gen 5 (not sure about prior). I'll check the others today.

[Marty-D]

The thing about Update is it triggers almost all the time. Even if you prevent these Berries from activating on the self-hit from confusion, they'll just activate immediately after you next use a move or confusion ends. The Berries shouldn't activate until your HP is next updated by an attack or something.

Also re: the test, the only reason it passes is because Swagger can miss, Deoxys might not hit itself in confusion, and even if it does, it knocks itself out in one hit at +2 so the Sitrus Berry is never used.

[QuiteQuiet]

Does this work better then? I can't think of anything else beside doing something with damage events.

Also re: the test, the only reason it passes is because Swagger can miss, Deoxys might not hit itself in confusion, and even if it does, it knocks itself out in one hit at +2 so the Sitrus Berry is never used.

Will this be enough? Test PRNG will hit + self-hit in confusion on the first turn, so it shouldn't be able to pass due to luck.
0 Atk Deoxys-Attack Struggle vs. 0 HP / 0 Def Deoxys-Attack: 150-177 (62.2 - 73.4%) -- guaranteed 2HKO

[Marty-D]

The confused check for Figy and friends seems to be the opposite of what you want but otherwise this looks like a great solution! 👍

[QuiteQuiet]

rip did

Still wondering about Oran Berry/Berry Juice/etc. though.

[Zarel]

So what's wrong with listening to damage events?

[QuiteQuiet]

If you mean onDamage, it fails several unrelated tests (as well as the one I just added for this) because it runs too early (I think):

 1) Emergency Exit should not request switch-out if first healed by berry:

     AssertionError [ERR_ASSERTION]: 'switch' === 'move'
     + expected - actual

     -switch
     +move

 2) Pickup should pick up a consumed item:
    AssertionError [ERR_ASSERTION]: Pick Up should retrieve consumed Sitrus Berry

 3) Sitrus Berry should heal 25% hp when consumed:
    AssertionError [ERR_ASSERTION]: Expected 'holdsItem' assertion to fail.

 4) Heal Block [Gen 4] should allow HP recovery items to activate:

     AssertionError [ERR_ASSERTION]: 'sitrusberry' === ''
     + expected - actual

     -sitrusberry

And if you meant onAfterDamage, this makes direct damage like Substitute (and various other things that uses directDamage) not trigger the berries at all. I don't know of any other damage event that could be reasonable, but neither of those work without breaking something different.

[Zarel]

We could fix onAfterDamage...

[QuiteQuiet]

Would it not be better to make directDamage trigger onAfterDamage (unless this was what you meant)? Or is there a reason why it doesn't at the moment.

[Zarel]

Yes, that's what I was proposing.

[DaWoblefet]

@Marty or @DaWoblefet can either of you confirm if Oran Berry/Berry Juice/Stat-boosting berries also does this or if it's only the healing berries

Here's my testing (if you need the video for confirmation / how the interaction works, let me know).

• Oran Berry - does not activate after confusion
• Berry Juice - does not activate after confusion
• Stat pinch Berries - does not activate after confusion
• Berserk - does not activate after confusion
• Emergency Exit / Wimp Out - does not activate after confusion. Subsequent attacks that would damage the Emergency Exit / Wimp Out Pokemon below 50% do not activate the ability.
  .
• Zen Mode - activates normally
• Schooling - activates normally
• Shields Down - activates normally
• Soul Heart (if a Pokemon faints to confusion) - activates normally
• Defeatist - halves the user's stats normally

[QuiteQuiet]

With onAfterDamage, the remaining problem from what I can see is that non-damage events (like Harvest harvesting a berry) won't then continue to trigger the Berry, which I think is the intended mechanic. There may be a way to work around that with some other event maybe?

[Zarel]

Attach it to onStart too?

[QuiteQuiet]

Okay, this might work then. I'm not entirely sure on the activation order after Recycle Air Balloon, but otherwise this should be good.

[Marty-D]

There's at least one regression with this approach. If you're facing something with Unnerve and it knocks you into range for your Berry to activate, then it switches out, your Berry wouldn't activate immediately anymore. (In fact, it should activate even more immediately than it does currently since switching to another Unnerve Pokemon stops the Berry from activating, which is itself a bug.)

[QuiteQuiet]

That's getting to a quite a lot of different events to consider... How many other likely regressions will come from this (Does Magic Room running out make a Pokemon eat a Berry straight away if they are in range for example?).

I guess an option would be to activate the onStart from the end events of the respective source that blocks items like this, but from what I see these events run before the switch actually happen, which would lead to berries being eaten too early instead...

onEnd: function (pokemon) {
	for (let i = 0; i < pokemon.side.foe.active.length; i++) {
		let active = pokemon.side.foe.active[i];
		let item = active.getItem();
		this.singleEvent('Start', item, active.itemData, active, pokemon, item);
	}
},

[pyuk-bot]

I wonder what happens if a Pokemon knocks itself below half through confusion, keeping its berry, and then something with Unnerve switches in and out before the Pokemon with the uneaten berry takes damage, or if the confusion hit happens while the berry is being negated through Unnerve, and then the Unnerve mon switches.

[Zarel]

At this point, I think the approach of "Update event, don't activate if previous event was confusion" might be the best approach.

But I'm starting to wonder how exactly Game Freak managed this bug. What happens if you hurt yourself in confusion, and then an opposing Unnerve user switches out?

[QuiteQuiet]

I'm honestly not sure what the best way around this is, and there's some interactions that I can't find information on (Magic Room expire after you hit yourself in confusion, do you eat the berry?, or Zarel's Unnerve situation).

[Marty-D]

(Magic Room expire after you hit yourself in confusion, do you eat the berry?)

I wonder what happens if a Pokemon knocks itself below half through confusion, keeping its berry, and then something with Unnerve switches in and out before the Pokemon with the uneaten berry takes damage, or if the confusion hit happens while the berry is being negated through Unnerve, and then the Unnerve mon switches.

All of these scenarios trigger the Berry.

[QuiteQuiet]

I don't think this will lead to any regressions (finally), but it doesn't fix the Unnerve into Unnerve problem Marty brought up earlier.

[Marty-D]

Could lastEffect ever be null here? Like say you just switched in and your HP is already in range. Maybe return early like you've done for the rest of the Berry checks just to be extra safe.

[QuiteQuiet]

It could, probably, so yeah probably better to move it outside.

[QuiteQuiet]

Does a Pokemon under the effects of Embargo that switches out eat its berry? I think I've covered the other cases now, but I'm not sure about that one.

[Marty-D]

When it comes back in later, yeah. Not during the switch or anything.

[QuiteQuiet]

This should cover everything then.

[Marty-D]

I think Klutz needs an onEnd also?

[QuiteQuiet]

Oh, right. Did.

[Marty-D]

onAfterEnd -> onEnd for Klutz; onAfterEnd won't work for Abilities.

The last thing would be to include Berry Juice in all your checks since it's not actually a Berry.

[QuiteQuiet]

During the onEnd event Klutz is still active so it will always end with pokemon.ignoringItem() being true. The onEnd as a whole in that regard doesn't work for anything here, which is why the onAfterEnd exist at all.

[Marty-D]

I just tested it; using your code, Super Fang to Gastro Acid didn't activate the Berry, but when I change it to onEnd it works.

[QuiteQuiet]

I was testing with Super Fang to Simple Beam, which doesn't work with onEnd, but onAfterEnd does. I guess I could be doing both (or find out what's happening exactly), but just onEnd isn't enough.

[Marty-D]

Oh, weird. I'll have to look into that later. I assume ignoringAbility() is checked in hasAbility(), which lets Gastro Acid work.

[QuiteQuiet]

From what I can tell, the Gastro Acid volatile is applied and then the ability onEnd runs. onEnd checks ignoringItem(), which does this.hasAbility('klutz'), and hasAbility checks for Gastro Acid.

On the other hand, when an ability ends in setAbility the onEnd event runs before the ability is replaced, so in this case Klutz is still active during the onEnd event.

Moving https:/Zarel/Pokemon-Showdown/blob/master/sim/pokemon.js#L1032 to the line above the singleEvent('End') call seems like a viable alternative alongside only the onEnd for Klutz, but I'm not sure if that would cause any unintended side-effects (the event is already called based on the old ability and ability data).

edit: apparently this messes with the activation order of things (berries are eaten before klutz properly ends, so probably not ideal either.

[Marty-D]

Hmm, I sprinkled exceptions to AfterEnd all over singleEvent() and nothing helped. I'm missing something here; any ideas, @Zarel?

[Zarel]

I'm confused. Can you describe the problem precisely?

[Marty-D]

Using onEnd for Klutz to make Berries activate immediately only works for the case where Gastro Acid stops Klutz from working, and onAfterEnd works for cases where Klutz gets replaced by some other Ability. How to make one of said events work for all cases?

[Zarel]

I would add a new item event, StartUpdate and trigger it before Start for items, as well as after Klutz. I would probably trigger it from onAfterEnd for Klutz, and figure out why it's not being triggered by Gastro Acid, and fix that (it's probably just missing a manual trigger).

I'd also consider this sort of change outside the scope of a single pull request, and split it into two.

[QuiteQuiet]

I believe I sorted out the Gastro Acid not properly triggering the onAfterEnd event for Klutz now, while still working for every other case too.

[Marty-D]

Oh, that's what I was missing, I never looked at Gastro Acid itself. Nice work, thanks! 👍