Can't be activated effects - fixed that some restricted effects show abilities as playable (example: Sharkey, Tyrant of the Shire and Karakas, fixed #10642)

Mage.Common/src/main/java/mage/view/SimpleCardView.java

@@ -22,7 +22,7 @@ public class SimpleCardView implements Serializable, SelectableObjectView {
 
     protected boolean isChoosable;
     protected boolean isSelected;
-    protected PlayableObjectStats playableStats = new PlayableObjectStats();
+    protected PlayableObjectStats playableStats = new PlayableObjectStats(); // filled on client side from GameView
 
     public SimpleCardView(final SimpleCardView view) {
         this.id = view.id;

Mage.Sets/src/mage/cards/k/Karakas.java

@@ -30,9 +30,9 @@ public final class Karakas extends CardImpl {
         super(ownerId, setInfo, new CardType[]{CardType.LAND}, "");
         this.supertype.add(SuperType.LEGENDARY);
 
-        // {tap}: Add {W}.
+        // {T}: Add {W}.
         this.addAbility(new WhiteManaAbility());
-        // {tap}: Return target legendary creature to its owner's hand.
+        // {T}: Return target legendary creature to its owner's hand.
         Ability ability = new SimpleActivatedAbility(Zone.BATTLEFIELD, new ReturnToHandTargetEffect(), new TapSourceCost());
         ability.addTarget(new TargetCreaturePermanent(filter));
         this.addAbility(ability);

Mage.Tests/src/test/java/org/mage/test/serverside/AbilityPickerTest.java

@@ -4,14 +4,23 @@ import mage.abilities.Abilities;
 import mage.abilities.Ability;
 import mage.cards.repository.CardInfo;
 import mage.cards.repository.CardRepository;
+import mage.constants.PhaseStep;
+import mage.constants.Zone;
 import mage.game.permanent.PermanentCard;
 import mage.game.permanent.PermanentImpl;
 import mage.view.AbilityPickerView;
 import mage.view.GameView;
+import mage.view.SimpleCardView;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mage.test.serverside.base.CardTestPlayerBase;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
 /**
  * @author JayDi85
  */
@@ -89,4 +98,74 @@ public class AbilityPickerTest extends CardTestPlayerBase {
         PermanentImpl permanent = new PermanentCard(info.createCard(), playerA.getId(), currentGame);
         return permanent.getAbilities(currentGame);
     }
+
+    @Test
+    public void test_RealGame_ActivatedAbilities_All() {
+        // possible bug: wrongly enabled ability, see #10642
+
+        // Activated abilities of lands your opponents control can't be activated unless they're mana abilities.
+        addCard(Zone.BATTLEFIELD, playerA, "Sharkey, Tyrant of the Shire", 1);
+        // {T}: Add {W}.
+        // {T}: Return target legendary creature to its owner's hand.
+        addCard(Zone.BATTLEFIELD, playerA, "Karakas", 1);
+
+        // have all abilities
+        runCode("all available", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> {
+            List<String> needList = new ArrayList<>(Arrays.asList(
+                    "{T}: Add {W}.",
+                    "{T}: Return target legendary creature to its owner's hand."
+            ));
+            Collections.sort(needList);
+            assertPlayableAbilities(needList);
+        });
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+    }
+
+    @Test
+    public void test_RealGame_ActivatedAbilities_Restricted() {
+        // possible bug: wrongly enabled ability, see #10642
+
+        // Activated abilities of lands your opponents control can't be activated unless they're mana abilities.
+        addCard(Zone.BATTLEFIELD, playerB, "Sharkey, Tyrant of the Shire", 1);
+        // {T}: Add {W}.
+        // {T}: Return target legendary creature to its owner's hand.
+        addCard(Zone.BATTLEFIELD, playerA, "Karakas", 1);
+
+        // have mana abilities only
+        runCode("non-mana ability disabled", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> {
+            List<String> needList = new ArrayList<>(Collections.singletonList(
+                    "{T}: Add {W}."
+            ));
+            Collections.sort(needList);
+            assertPlayableAbilities(needList);
+        });
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+    }
+
+    private void assertPlayableAbilities(List<String> need) {
+        // server side
+        List<String> realList = playerA.getPlayable(currentGame, true).stream()
+                .map(Ability::getRule)
+                .sorted()
+                .collect(Collectors.toList());
+        Assert.assertEquals("wrong server side playable list", need.toString(), realList.toString());
+
+        // client side as game data
+        GameView gameView = getGameView(playerA);
+        realList.clear();
+        gameView.getCanPlayObjects().getObjects().forEach((objectId, stats) -> {
+            stats.getPlayableAbilityIds().forEach(abilityId -> {
+                Ability ability = currentGame.getAbility(abilityId, objectId).orElse(null);
+                realList.add(ability == null ? "null" : ability.getRule());
+            });
+        });
+        Collections.sort(realList);
+        Assert.assertEquals("wrong client side playable list", need.toString(), realList.toString());
+    }
 }

Mage/src/main/java/mage/abilities/ActivatedAbilityImpl.java

@@ -10,6 +10,7 @@ import mage.cards.Card;
 import mage.constants.*;
 import mage.game.Game;
 import mage.game.command.CommandObject;
+import mage.game.events.GameEvent;
 import mage.game.permanent.Permanent;
 import mage.players.Player;
 import mage.util.CardUtil;
@@ -138,6 +139,13 @@ public abstract class ActivatedAbilityImpl extends AbilityImpl implements Activa
             return ActivationStatus.getFalse();
         }
 
+        // activate restrictions by replacement effects (example: Sharkey, Tyrant of the Shire)
+        if (this.isActivatedAbility()) {
+            if (game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.ACTIVATE_ABILITY, this.getId(), this, playerId))) {
+                return ActivationStatus.getFalse();
+            }
+        }
+
         // all fine, can be activated
         // TODO: WTF, must be rework to remove data change in canActivate call
         //  (it can be called from any place by any player or card).

Mage/src/main/java/mage/abilities/icon/system/PlayableCountIcon.java

@@ -26,9 +26,9 @@ public final class PlayableCountIcon extends CardIconImpl {
     private static String getHint(PlayableObjectStats objectStats) {
         String res = "Playable abilities: " + objectStats.getPlayableAmount();
         // abilities list already sorted
-        List<String> list = objectStats.getPlayableAbilities();
+        List<String> list = objectStats.getPlayableAbilityNames();
         final int[] counter = {0};
-        if (list.size() > 0) {
+        if (!list.isEmpty()) {
             res += "<br>" + list
                     .stream()
                     .map(s -> {

Mage/src/main/java/mage/game/events/GameEvent.java

@@ -247,6 +247,7 @@ public class GameEvent implements Serializable {
          */
         ACTIVATE_ABILITY, ACTIVATED_ABILITY,
         /* ACTIVATE_ABILITY, ACTIVATED_ABILITY,
+         WARNING, do not use choose dialogs inside, can be calls multiple types, e.g. on playable checking
          targetId    id of the ability to activate / use
          sourceId    sourceId of the object with that ability
          playerId    player that tries to use this ability

Mage/src/main/java/mage/players/PlayableObjectStats.java

@@ -55,11 +55,11 @@ public class PlayableObjectStats implements Serializable, Copyable<PlayableObjec
             // collect info about abilities for card icons popup, must be simple online text (html symbols are possible)
             // some long html tags can be miss (example: ability extra hint) -- that's ok
             String shortInfo = ability.toString();
+            shortInfo = shortInfo.replace("<br>", " ");
+            shortInfo = shortInfo.replace("\n", " ");
             if (shortInfo.length() > 50) {
                 shortInfo = shortInfo.substring(0, 50 - 1) + "...";
             }
-            shortInfo = shortInfo.replace("<br>", " ");
-            shortInfo = shortInfo.replace("\n", " ");
             dest.add(new PlayableObjectRecord(ability.getId(), shortInfo));
         }
     }
@@ -91,7 +91,7 @@ public class PlayableObjectStats implements Serializable, Copyable<PlayableObjec
                 + this.other.size();
     }
 
-    public List<String> getPlayableAbilities() {
+    public List<String> getPlayableAbilityNames() {
         List<String> all = new ArrayList<>();
         all.addAll(this.basicManaAbilities.stream().map(PlayableObjectRecord::getValue).sorted().collect(Collectors.toList()));
         all.addAll(this.basicPlayAbilities.stream().map(PlayableObjectRecord::getValue).sorted().collect(Collectors.toList()));
@@ -100,6 +100,15 @@ public class PlayableObjectStats implements Serializable, Copyable<PlayableObjec
         return all;
     }
 
+    public List<UUID> getPlayableAbilityIds() {
+        List<UUID> all = new ArrayList<>();
+        all.addAll(this.basicManaAbilities.stream().map(PlayableObjectRecord::getId).collect(Collectors.toList()));
+        all.addAll(this.basicPlayAbilities.stream().map(PlayableObjectRecord::getId).collect(Collectors.toList()));
+        all.addAll(this.basicCastAbilities.stream().map(PlayableObjectRecord::getId).collect(Collectors.toList()));
+        all.addAll(this.other.stream().map(PlayableObjectRecord::getId).collect(Collectors.toList()));
+        return all;
+    }
+
     public int getPlayableImportantAmount() {
         // return only important abilities (e.g. show it as card icons)
         return this.other.size();

Mage/src/main/java/mage/players/PlayableObjectsList.java

@@ -4,10 +4,7 @@ import mage.abilities.ActivatedAbility;
 import mage.util.Copyable;
 
 import java.io.Serializable;
-import java.util.HashMap;
+import java.util.*;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
 
 /**
  * Contains stats with all playable cards for the player
@@ -66,4 +63,8 @@ public class PlayableObjectsList implements Serializable, Copyable<PlayableObjec
             return 0;
         }
     }
+
+    public Map<UUID, PlayableObjectStats> getObjects() {
+        return objects;
+    }
 }

Mage/src/main/java/mage/players/PlayerImpl.java

@@ -1447,10 +1447,7 @@ public abstract class PlayerImpl implements Player, Serializable {
         }
     }
 
-
     protected boolean playManaAbility(ActivatedManaAbilityImpl ability, Game game) {
-        if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.ACTIVATE_ABILITY,
-                ability.getId(), ability, playerId))) {
         int bookmark = game.bookmarkState();
         if (ability.activate(game, false) && ability.resolve(game)) {
             if (ability.isUndoPossible()) {
@@ -1463,16 +1460,14 @@ public abstract class PlayerImpl implements Player, Serializable {
             return true;
         }
         restoreState(bookmark, ability.getRule(), game);
-        }
         return false;
     }
 
     protected boolean playAbility(ActivatedAbility ability, Game game) {
         //20091005 - 602.2a
-        if (ability.isUsesStack()) {
-            if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.ACTIVATE_ABILITY,
-                    ability.getId(), ability, playerId))) {
         int bookmark = game.bookmarkState();
+        if (ability.isUsesStack()) {
+            // put to stack
             setStoredBookmark(bookmark); // move global bookmark to current state (if you activated mana before then you can't rollback it)
             ability.newId();
             ability.setControllerId(playerId);
@@ -1488,17 +1483,16 @@ public abstract class PlayerImpl implements Player, Serializable {
                 return true;
             }
             restoreState(bookmark, ability.getRule(), game);
-            }
         } else {
-            int bookmark = game.bookmarkState();
+            // resolve without stack
             if (ability.activate(game, false)) {
                 ability.resolve(game);
                 game.removeBookmark(bookmark);
                 resetStoredBookmark(game);
                 return true;
             }
-            restoreState(bookmark, ability.getRule(), game);
         }
+        restoreState(bookmark, ability.getRule(), game);
         return false;
     }