Problems with cost implementations
- Dominant language
- Java
- Stars
- 2.4k
- Forks
- 940
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 160
Description
**1 - Miss notTarget**
Miss `notTarget = true` setting in target's usage (all costs must be non targeted, e.g. unaffected by protection/hexproof/etc). Original bug for [[Halo Fountain]] in #10559, but other classes also affected.
How-to fix:
* search code by `extends CostImpl`
* check added target for `notTarget` param;
* bug example with [[Gurzigost]]:


**2 - miss usage of targets field**
Not a bug, but a good code style: `CostImpl` already has `targets` field, so it's a good practice to add a target class to that list instead a custom field
Bad usage in `UntapTargetCost`:

Good usage in [[Gurzigost]]:

**3 - miss clear paid code** (~~need research~~)
Maybe not a bug, but it bad smells. `CostImpl` uses `paid` field by default, but some implementation can use custom status from targets, lists, etc -- so that lists must be (?) cleared before usage.
If you look to `DiscardTargetCost` then can find:
* `this.targets.clearChosen();` in `pay` method;
* override `clearPaid()` method with `this.targets.clearChosen()` call
I run tests and catch breakpoint with filled values in `DiscardTargetCost` class
Condition: `this.targets.size() > 0 && this.targets.getFirstTarget() != null`

Possible reason of filled targets:
* it filled before by AI simulations (popular bug with shared data);
* it filled before by design (e.g. multiple calls of pay methods);
* it filled before by wrong choose (player cancel or choose a wrong card) -- well, I think it's a real reason for clear targets usage:


So other custom cost can be affected same bug and it's a good practice to call `this.targets.clearChosen();` all the time (in `pay` and ~~`clearPaid` methods~~ -- the last one fixed by 476fd27f65e01910619ecfcbc6c97cefb9b891f8).
Contributor guide
No contributing guide indexed for this repository
Research direction
Search for classes extending CostImpl, then inspect target handling in the affected cost implementations, including UntapTargetCost and DiscardTargetCost. Review the existing tests and run the relevant test suite while checking target flags, use of CostImpl.targets, and clearing of chosen targets. Done means the remaining affected implementations consistently handle these cases without regressions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- game-dev
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100