magefree / magefree/mage

Ability::getTargets() wrongly used in some use cases (unsupport multi-modes spells/cards)

Open
#12,227 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
2.4k
Forks
940
Avg merge
2d 12h
Merged PRs (30d)
160

Description

`Ability::getTargets()` do not work with multi modes abilities and return only first mode's targets (it's about ability's targets, not cost's targets).

![shot_240506_082822](https://github.com/magefree/mage/assets/8344157/a885d91c-8c75-4584-b032-e08c973485cb)

It's works fine in 95% cause most of the usages work with a single mode cards. But it will fail on usage with 2+ mode cards.

Usages examples:
1. Update dynamic targets (search by `getSpellAbility().getTargets().clear()`):
* ![shot_240506_084017](https://github.com/magefree/mage/assets/8344157/d546fec0-51be-456e-b8b8-e2af4e474f83)
2. Access to targets data like filters, selected ids, etc (search by `.getTargets().get(`):
* ![shot_240506_084534](https://github.com/magefree/mage/assets/8344157/bccbab2d-6a97-4d07-889e-27f25af9b3e7)
* ![shot_240506_092631](https://github.com/magefree/mage/assets/8344157/209dd8a3-6ad1-4ca4-ad33-90e415f83208)

Some hints on good usage:
* if you're sure there are a predefined targets then can use index/first access (current usages);
* if you work with unknown abilities/targets then use:
* List of all selected target ids: `CardUtil.getAllSelectedTargets(ability, game)`
* List of all possible target ids: `CardUtil.getAllPossibleTargets(ability, game)`
* List of all target classes (if you need filters, canTarget, etc): `Ability::getAllSelectedTargets()`

What can be fixed/refactored:
* [ ] search and fix of `getStackAbility().getTargets()` (some use cases must use all target ids by `CardUtil.getAllSelectedTargets(ability, game)`):
![shot_240506_091923](https://github.com/magefree/mage/assets/8344157/b227edf8-a9b2-4f41-a6c2-ee0229ba26de)
* [ ] need research and find another use cases to fix;

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by searching for getStackAbility().getTargets() and other getTargets() usages, then compare each use with CardUtil.getAllSelectedTargets(ability, game), CardUtil.getAllPossibleTargets(ability, game), and Ability::getAllSelectedTargets(). Audit the relevant usages for multi-mode abilities and update those that need all target data; done means multi-mode spells and cards no longer lose targets while single-mode behavior remains correct.

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.