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

Replace "it.dmg.value" with "it.notZero" for multiple damage modifiers applied before W/R, plus some other fixes in those code sections #942

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

starg09
Copy link
Collaborator

@starg09 starg09 commented Sep 5, 2020

When finished, should fix some issues with cards such as "Darkness Energy vs Metal Energy" in wizards formats, or "Professor Kukui vs Pressure/Intimidating_Fang" in recent formats.

Counts as a mass replacement, so leaving a checklist for looked-around gens:

  • Gen 1 [Groovy reimplementations]
  • Gen 2
  • Gen 3 [Sets in Groovy implementation]
  • Gen 4 [What's implemented here, DP-CL]
  • Gen 5 [Cards using Groovy implementation]
  • Gen 6 [Cards using Groovy implementation]
  • Gen 7
  • Gen 8

Sets not entirely fixable this way:

  • Gen 1 [Legacy implementation]
  • Gen 3 [Sets in Legacy implementation, RS-HL]
  • Gen 5 [Effects that may use legacy code]
  • Gen 6 [Effects that may use legacy code]

* Intimidating Pattern/Fang/Pressure abilities are no longer ignored by swift attacks. It's an effect on the attacker, not on the defending Pokémon.
* Minor logic changes in some cards.
@starg09 starg09 marked this pull request as ready for review September 24, 2020 00:06
@axpendix
Copy link
Owner

Hi @flagrama could you review this one please?

Copy link
Collaborator

@flagrama flagrama left a comment

Choose a reason for hiding this comment

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

Request to put a comment like

//Not checking for it.notNoEffect since it's applied before W/R, should count as an effect on the attacking pokemon.

Somewhere in the PR description or comments when it's done all over the place within the PR. 😆

Requesting a lot of bcs to be changed for consistency. If we want to clarify them at all, I think it'd be better to get them all at once so that a player doesn't get confused when some effects say it one way, and some say it the other.

A couple of questions about a few of the removed it.notNoEffects as well. Just need clarification that they are working as intended.

src/tcgwars/logic/impl/gen3/Deoxys.groovy Outdated Show resolved Hide resolved
src/tcgwars/logic/impl/gen3/Deoxys.groovy Outdated Show resolved Hide resolved
src/tcgwars/logic/impl/gen3/HolonPhantoms.groovy Outdated Show resolved Hide resolved
src/tcgwars/logic/impl/gen4/HeartgoldSoulsilver.groovy Outdated Show resolved Hide resolved
before APPLY_ATTACK_DAMAGES, {
if (bg.currentTurn == self.owner){
bg.dm().each {
if(it.to == defending && it.dmg.value && it.notNoEffect) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's definitely correct to do this even with an attack that ignores effects on the Defending Pokémon?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd go with "it doesn't care", since it's an effect entirely within the defending Pokémon itself. It only cares about being damaged, doesn't affect the damage itself in any way.

As an example from Japan (couldn't find a good one in Compendium):

If, after playing the Supporter "Phoebe", I use my Pokémon VMAX attack and deal more damage than the HP of my opponent's Aggron with the "Sturdy" ability, will the opponent's Aggron be knocked out?

No, it will not.
In this case, the Aggron will remain in the field with 10 HP remaining.

As a similar case, a Reshiram&Charizard-GX using full-effect Double Blaze GX would still get 2 damage counters applied, if attacking a Pokémon with a Rocky Helmet attached.

src/tcgwars/logic/impl/gen7/ForbiddenLight.groovy Outdated Show resolved Hide resolved
src/tcgwars/logic/impl/gen7/ForbiddenLight.groovy Outdated Show resolved Hide resolved
src/tcgwars/logic/impl/gen7/LostThunder.groovy Outdated Show resolved Hide resolved
src/tcgwars/logic/impl/gen7/SunMoonPromos.groovy Outdated Show resolved Hide resolved
@starg09 starg09 requested a review from flagrama May 27, 2021 15:21
@starg09 starg09 marked this pull request as draft May 27, 2021 15:24
@starg09
Copy link
Collaborator Author

starg09 commented May 27, 2021

Turning into draft as there's now many gen 4 sets implemented, which may need updates in this regard.

starg09 added 5 commits May 27, 2021 19:09
Should eventually be turned into a ruleset-dependant single trainer for DP/HGSS/BW, but for now should work fine (HGSS original or errata usage is still not confirmed, for the HGSS block)
Should affect any damage, not only to owner's Pokémon.
These all include more substantial changes than just the replacement, so worth taking a separate peek for sure.
@starg09 starg09 marked this pull request as ready for review May 27, 2021 23:43
@starg09 starg09 marked this pull request as draft October 3, 2023 13:17
@axpendix
Copy link
Owner

Hi @starg09 I hope you're well! Is there any chance for you to return and fix this one? :) btw, flagrama is already back! Cheers

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.

3 participants