linking refactor

src/data/action/set.cpp

@@ -22,66 +22,74 @@ AddCardAction::AddCardAction(Set& set)
   , action(ADD, make_intrusive<Card>(*set.game), set.cards)
 {}
 
-AddCardAction::AddCardAction(AddingOrRemoving ar, Set& set, const CardP& card)
-  : CardListAction(set)
-  , action(ar, card, set.cards)
-{}
-
 AddCardAction::AddCardAction(AddingOrRemoving ar, Set& set, const vector<CardP>& cards)
   : CardListAction(set)
   , action(ar, cards, set.cards)
-{}
+{
+  // If we are adding cards, resolve any uid conflicts
+  // We always assume uid conflicts occur because a card was copy-pasted into the same set,
+  // and never because two different cards randomly got assigned the same uid
+  if (!action.adding) return;
+  // Tally existing unique ids
+  unordered_map<String, CardP> all_existing_cards;
+  unordered_set<String> all_existing_uids;
+  FOR_EACH(card, set.cards) {
+    all_existing_cards.insert({ card->uid, card });
+    all_existing_uids.insert(card->uid);
+  }
+  // Tally added unique ids
+  unordered_map<String, CardP> all_added_uids;
+  for (size_t pos = 0; pos < action.steps.size(); ++pos) {
+    CardP card = action.steps[pos].item;
+    all_added_uids.insert({ card->uid, card });
+  }
+  // Cycle on the added cards
+  unordered_map<String, CardP> all_added_uids_copy(all_added_uids);
+  FOR_EACH(added_pair, all_added_uids_copy) {
+    String old_uid = added_pair.first;
+    CardP added_card = added_pair.second;
+    // Assign new unique id if necessary
+    if (all_existing_cards.find(old_uid) == all_existing_cards.end()) continue;
+    String new_uid = generate_uid();
+    added_card->uid = new_uid;
+    all_added_uids.insert({ new_uid, added_card });
+    // Update links on linked cards
+    LINK_PAIRS(linked_pairs, added_card);
+    FOR_EACH(linked_pair, linked_pairs) {
+      String& linked_uid = linked_pair.first.get();
+      String& linked_relation = linked_pair.second.get();
+      if (linked_uid.empty()) continue;
+      // If it's an added card, replace the link
+      if (all_added_uids.find(linked_uid) != all_added_uids.end()) {
+        all_added_uids.at(linked_uid)->updateLinkedUID(old_uid, new_uid);
+      }
+      // Otherwise, if it's an existing card, create an action to copy the link
+      else if (all_existing_cards.find(linked_uid) != all_existing_cards.end()) {
+        CardP linked_card = all_existing_cards.at(linked_uid);
+        int linked_index = linked_card->findFreeLink(new_uid, all_existing_uids);
+        if (linked_index < 0) {
+          queue_message(MESSAGE_WARNING, _ERROR_1_("not enough free links", linked_card->identification()));
+        }
+        else {
+          String relation(linked_card->getLinkedRelation(linked_index));
+          card_link_actions.push_back(make_intrusive<OneWayLinkCardsAction>(set, linked_card, new_uid, relation, linked_index));
+        }
+      }
+    }
+  }
+}
 
 String AddCardAction::getName(bool to_undo) const {
   return action.getName();
 }
 
 void AddCardAction::perform(bool to_undo) {
-  // If we are adding cards, resolve any uid conflicts
-  // (If we are re-adding cards, from a remove undo, there shouldn't be any uid conflicts)
-  // We always assume uid conflicts occur because a card was copy-pasted into the same set,
-  // and never because two different cards randomly got assigned the same uid
-  if (action.adding && !to_undo) {
-    // Tally existing unique ids
-    unordered_map<String, CardP> all_existing_uids;
-    FOR_EACH(card, set.cards) {
-      all_existing_uids.insert({ card->uid, card });
-    }
-    // Tally added unique ids
-    unordered_map<String, CardP> all_added_uids;
-    for (size_t pos = 0; pos < action.steps.size(); ++pos) {
-      CardP card = action.steps[pos].item;
-      all_added_uids.insert({ card->uid, card });
-    }
-    FOR_EACH(added_pair, all_added_uids) {
-      String old_uid = added_pair.first;
-      CardP added_card = added_pair.second;
-      // Assign new unique ids
-      if (all_existing_uids.find(old_uid) != all_existing_uids.end()) {
-        String new_uid = generate_uid();
-        added_card->uid = new_uid;
-        all_added_uids.insert({ new_uid, added_card });
-        // Update links on linked cards
-        OTHER_LINKED_PAIRS(linked_pairs, added_card);
-        FOR_EACH(linked_pair, linked_pairs) {
-          String& linked_uid = linked_pair.first.get();
-          String& linked_relation = linked_pair.second.get();
-          if (linked_uid.empty()) continue;
-          // If it's an added card, replace the link
-          if (all_added_uids.find(linked_uid) != all_added_uids.end()) {
-            all_added_uids.at(linked_uid)->updateLink(old_uid, new_uid);
-          }
-          // Otherwise, if it's an existing card, copy the link
-          else if (all_existing_uids.find(linked_uid) != all_existing_uids.end()) {
-            all_existing_uids.at(linked_uid)->copyLink(set, old_uid, new_uid);
-          }
-        }
-      }
-    }
-  }
-
   // Add or remove cards
   action.perform(set.cards, to_undo);
+  // Update card links
+  for (int i = 0; i < card_link_actions.size(); ++i) {
+    card_link_actions[i]->perform(to_undo);
+  }
 }
 
 // ----------------------------------------------------------------------------- : Reorder cards
@@ -108,39 +116,40 @@ void ReorderCardsAction::perform(bool to_undo) {
 
 // ----------------------------------------------------------------------------- : Link cards
 
-LinkCardsAction::LinkCardsAction(Set& set, const CardP& selected_card, vector<CardP>& linked_cards, const String& selected_relation, const String& linked_relation)
-  : CardListAction(set), selected_card(selected_card), linked_cards(linked_cards), selected_relation(selected_relation), linked_relation(linked_relation)
-{}
+OneWayLinkCardsAction::OneWayLinkCardsAction(Set& set, CardP& card, const String& uid, const String& relation, int index)
+  : CardListAction(set), card(card), uid(uid), relation(relation)
+{
+  switch (index) {
+    case 1: {
+      linked_uid      = &card->linked_card_1;
+      linked_relation = &card->linked_relation_1;
+      return;
+    }
+    case 2: {
+      linked_uid      = &card->linked_card_2;
+      linked_relation = &card->linked_relation_2;
+      return;
+    }
+    case 3: {
+      linked_uid      = &card->linked_card_3;
+      linked_relation = &card->linked_relation_3;
+      return;
+    }
+    default: {
+      linked_uid      = &card->linked_card_4;
+      linked_relation = &card->linked_relation_4;
+      return;
+    }
+  }
+}
 
-String LinkCardsAction::getName(bool to_undo) const {
-  return _("Link cards");
+String OneWayLinkCardsAction::getName(bool to_undo) const {
+  return _("Change link");
 }
 
-void LinkCardsAction::perform(bool to_undo) {
-  if (!to_undo) {
-    selected_card->link(set, linked_cards, selected_relation, linked_relation);
-  } else {
-    selected_card->unlink(linked_cards);
-  }
-}
-
-UnlinkCardsAction::UnlinkCardsAction(Set& set, const CardP& selected_card, CardP& unlinked_card)
-  : CardListAction(set), selected_card(selected_card), unlinked_card(unlinked_card)
-{}
-
-String UnlinkCardsAction::getName(bool to_undo) const {
-  return _("Unlink card");
-}
-
-void UnlinkCardsAction::perform(bool to_undo) {
-  if (!to_undo) {
-    pair<String, String> relations = selected_card->unlink(unlinked_card);
-    selected_relation = relations.first;
-    unlinked_relation = relations.second;
-  }
-  else {
-    selected_card->link(set, unlinked_card, selected_relation, unlinked_relation);
-  }
+void OneWayLinkCardsAction::perform(bool to_undo) {
+  swap(*linked_uid,      uid);
+  swap(*linked_relation, relation);
 }
 
 // ----------------------------------------------------------------------------- : Change stylesheet
@@ -236,7 +245,7 @@ String ChangeCardUIDAction::getName(bool to_undo) const {
 }
 void ChangeCardUIDAction::perform(bool to_undo) {
   FOR_EACH(c, set.cards) {
-    c->updateLink(card->uid, uid);
+    c->updateLinkedUID(card->uid, uid);
   }
   swap(card->uid, uid);
 }

src/data/action/set.hpp

@@ -41,13 +41,13 @@ class AddCardAction : public CardListAction {
 public:
   /// Add a newly allocated card
   AddCardAction(Set& set);
-  AddCardAction(AddingOrRemoving, Set& set, const CardP& card);
   AddCardAction(AddingOrRemoving, Set& set, const vector<CardP>& cards);
   
   String getName(bool to_undo) const override;
   void perform(bool to_undo) override;
   
-  const GenericAddAction<CardP> action;
+  const GenericAddAction<CardP> action;
+  vector<ActionP> card_link_actions;
 };
 
 // ----------------------------------------------------------------------------- : Reorder cards
@@ -67,32 +67,19 @@ public:
 // ----------------------------------------------------------------------------- : Link cards
 
 /// Add a link between two or more cards
-class LinkCardsAction : public CardListAction {
+class OneWayLinkCardsAction : public CardListAction {
 public:
-  LinkCardsAction(Set& set, const CardP& selected_card, vector<CardP>& linked_cards, const String& selected_relation, const String& linked_relation);
-  
+  OneWayLinkCardsAction(Set& set, CardP& card, const String& uid, const String& relation, int index);
+
   String getName(bool to_undo) const override;
   void perform(bool to_undo) override;
-  
+
   //private:
-  CardP         selected_card;     ///< The card currently selected in the cards tab
-  vector<CardP> linked_cards;      ///< The cards that will be linked to the selected card
-  String        selected_relation; ///< The nature of the relation of the selected card
-  String        linked_relation;   ///< The nature of the relation of the linked cards
-};
-/// Remove a link between two cards
-class UnlinkCardsAction : public CardListAction {
-public:
-    UnlinkCardsAction(Set& set, const CardP& selected_card, CardP& unlinked_card);
-  
-  String getName(bool to_undo) const override;
-  void perform(bool to_undo) override;
-  
-  //private:
-  CardP         selected_card;     ///< The card currently selected in the cards tab
-  CardP         unlinked_card;     ///< The card that will be unlinked from the selected card
-  String        selected_relation; ///< The nature of the relation of the selected card
-  String        unlinked_relation; ///< The nature of the relation of the unlinked card
+  CardP   card;            ///< The card we change the link of. We hold a reference so it doesn't get destroyed before this.
+  String* linked_uid;      ///< The uid slot we need to change
+  String* linked_relation; ///< The relation slot we need to change
+  String  uid;             ///< The uid we have to change the link to
+  String  relation;        ///< The relation we have to change the link to
 };
 
 // ----------------------------------------------------------------------------- : Change stylesheet

src/data/action/value.cpp

@@ -212,12 +212,12 @@ void ScriptStyleEvent::perform(bool) {
 
 // ----------------------------------------------------------------------------- : Bulk action
 
-BulkAction::BulkAction(const vector<shared_ptr<Action>>& actions, const SetP& set, CardListBase* card_list_window)
+BulkAction::BulkAction(const vector<ActionP>& actions, const SetP& set, CardListBase* card_list_window)
   : actions(actions), set(set), card_list_window(card_list_window)
 {
   if (actions.empty()) throw ScriptError(_("BulkAction created with no actions"));
-  name_do = actions.front()->getName(false) + _(" ") + _ACTION_("bulk");
-  name_undo = actions.front()->getName(true) + _(" ") + _ACTION_("bulk");
+  name_do = _ACTION_1_("bulk", actions.front()->getName(false));
+  name_undo = _ACTION_1_("bulk", actions.front()->getName(true));
 }
 BulkAction::~BulkAction() {}
 

src/data/action/value.hpp

@@ -206,7 +206,7 @@ public:
 // An action that's just a list of other actions
 class BulkAction : public Action {
 public:
-  BulkAction(const vector<shared_ptr<Action>>& actions, const SetP& set, CardListBase* card_list_window);
+  BulkAction(const vector<ActionP>& actions, const SetP& set, CardListBase* card_list_window);
   ~BulkAction() override;
 
   String getName(bool to_undo) const override;
@@ -216,7 +216,7 @@ public:
 private:
   String name_do;
   String name_undo;
-  vector<shared_ptr<Action>> actions;
+  vector<ActionP> actions;
   SetP set;
   CardListBase* card_list_window;
 };

src/data/card.cpp

@@ -63,205 +63,110 @@ bool Card::contains(QuickFilterPart const& query) const {
   }
   if (query.match(_("notes"), notes)) return true;
   return false;
-}
-
-void Card::link(const Set& set, const vector<CardP>& linked_cards, const String& selected_relation, const String& linked_relation)
-{
-  unlink(linked_cards);
-
-  unordered_set<String> all_existing_uids;
-  FOR_EACH(card, set.cards) {
-    all_existing_uids.insert(card->uid);
-  }
-  int free_link_count = 0;
-  THIS_LINKED_PAIRS(this_linked_pairs);
-  FOR_EACH(this_linked_pair, this_linked_pairs) {
-    String& this_linked_uid = this_linked_pair.first.get();
-    if (
-      this_linked_uid.empty() ||                                // Not a reference
-      all_existing_uids.find(this_linked_uid) == all_existing_uids.end() // Reference to nonexistent card
-      ) free_link_count++;
-  }
-  if (free_link_count < linked_cards.size()) {
-    queue_message(MESSAGE_WARNING, _ERROR_("not enough free links"));
-    return;
-  }
-
-  vector<CardP> all_missed_cards;
-  FOR_EACH(linked_card, linked_cards) {
-    bool written = false;
-    // Try to write to a free spot
-    FOR_EACH(this_linked_pair, this_linked_pairs) {
-      String& this_linked_uid = this_linked_pair.first.get();
-      String& this_linked_relation = this_linked_pair.second.get();
-      if (this_linked_uid.empty()) {
-        this_linked_uid = linked_card->uid;
-        this_linked_relation = linked_relation;
-        written = true;
-        break;
+}
+
+vector<int> Card::findFreeLinks(vector<String>& linked_uids, const unordered_set<String>& all_existing_uids) {
+  vector<int> freeIndexes;
+  int count = min(4, (int)linked_uids.size());
+  LINK_PAIRS(linked_pairs, this);
+
+  for (int i = 0; i < count; ++i) {
+    String& linked_uid = linked_uids[i];
+    // Try to find an existing link
+    for (int j = 0; j < linked_pairs.size(); ++j) {
+      auto& linked_pair = linked_pairs[j];
+      if (linked_pair.first.get() == linked_uid &&
+          std::find(freeIndexes.begin(), freeIndexes.end(), j) != freeIndexes.end()
+      ) {
+        freeIndexes.push_back(j);
+        goto continue_outer;
+      }
+    }
+    // Try to find a free spot
+    for (int j = 0; j < linked_pairs.size(); ++j) {
+      auto& linked_pair = linked_pairs[j];
+      if (linked_pair.first.get().empty() &&
+          std::find(freeIndexes.begin(), freeIndexes.end(), j) != freeIndexes.end()
+      ) {
+        freeIndexes.push_back(j);
+        goto continue_outer;
       }
     }
-    // Try to write to an erasable spot
-    if (!written) {
-      FOR_EACH(this_linked_pair, this_linked_pairs) {
-        String& this_linked_uid = this_linked_pair.first.get();
-        String& this_linked_relation = this_linked_pair.second.get();
-        if (all_existing_uids.find(this_linked_uid) == all_existing_uids.end()) {
-          this_linked_uid = linked_card->uid;
-          this_linked_relation = linked_relation;
-          written = true;
-          break;
-        }
+    // Try to find an erasable spot
+    for (int j = 0; j < linked_pairs.size(); ++j) {
+      auto& linked_pair = linked_pairs[j];
+      if (all_existing_uids.find(linked_pair.first.get()) == all_existing_uids.end() &&
+          std::find(freeIndexes.begin(), freeIndexes.end(), j) != freeIndexes.end()
+      ) {
+        freeIndexes.push_back(j);
+        goto continue_outer;
       }
-    }
-    if (!written) {
-      // Should be impossible to end up here?
-    }
+    }
+    freeIndexes.push_back(-1);
+    continue_outer:;
+  }
+  return freeIndexes;
+}
+int Card::findFreeLink(const String& linked_uid, const unordered_set<String>& all_existing_uids) {
+  vector<String> linked_uids { linked_uid };
+  return findFreeLinks(linked_uids, all_existing_uids)[0];
+}
+
+int Card::findUIDLink(const String& linked_uid) {
+  if (linked_card_1 == linked_uid) return 0;
+  if (linked_card_2 == linked_uid) return 1;
+  if (linked_card_3 == linked_uid) return 2;
+  if (linked_card_4 == linked_uid) return 3;
+  return -1;
+}
+
+vector<int> Card::findRelationLinks(const String& linked_relation) {
+  vector<int> indexes;
+  if (linked_relation_1 == linked_relation) indexes.push_back(0);
+  if (linked_relation_2 == linked_relation) indexes.push_back(1);
+  if (linked_relation_3 == linked_relation) indexes.push_back(2);
+  if (linked_relation_4 == linked_relation) indexes.push_back(3);
+  return indexes;
+}
+
+String& Card::getLinkedUID(int index) {
+  switch (index) {
+  case 3:  return linked_card_4;
+  case 2:  return linked_card_3;
+  case 1:  return linked_card_2;
+  default: return linked_card_1;
+  }
+}
+String& Card::getLinkedRelation(int index) {
+  switch (index) {
+  case 3:  return linked_relation_4;
+  case 2:  return linked_relation_3;
+  case 1:  return linked_relation_2;
+  default: return linked_relation_1;
+  }
+}
 
-    OTHER_LINKED_PAIRS(linked_pairs, linked_card);
-    written = false;
-    // Try to write to a free spot
-    FOR_EACH(linked_pair, linked_pairs) {
-      String& linked_uid = linked_pair.first.get();
-      String& linked_relation = linked_pair.second.get();
-      if (linked_uid.empty()) {
-        linked_uid = uid;
-        linked_relation = selected_relation;
-        written = true;
-        break;
-      }
-    }
-    // Try to write to an erasable spot
-    if (!written) {
-      FOR_EACH(linked_pair, linked_pairs) {
-        String& linked_uid = linked_pair.first.get();
-        String& linked_relation = linked_pair.second.get();
-        if (all_existing_uids.find(linked_uid) == all_existing_uids.end()) {
-          linked_uid = uid;
-          linked_relation = selected_relation;
-          written = true;
-          break;
-        }
-      }
-    }
-    // Notify we couldn't write
-    if (!written) {
-      all_missed_cards.push_back(linked_card);
-    }
-  }
-  if (all_missed_cards.size() > 0) {
-    std::stringstream ss;
-    ss << _ERROR_("could not link");
-    for (size_t pos = 0; pos < all_missed_cards.size(); ++pos) {
-      ss << all_missed_cards[pos]->identification();
-      if (pos < all_missed_cards.size() - 1) ss << ", ";
-    };
-    queue_message(MESSAGE_WARNING, wxString(ss.str().c_str()));
-  }
+void Card::updateLinkedUID(const String& old_uid, const String& new_uid) {
+  int index = findUIDLink(old_uid);
+  if (index >= 0) getLinkedUID(index) = new_uid;
 }
 
-void Card::link(const Set& set, CardP& linked_card, const String& selected_relation, const String& linked_relation)
-{
-  vector<CardP> linked_cards { linked_card };
-  link(set, linked_cards, selected_relation, linked_relation);
+vector<CardP> Card::getLinkedRelationCards(const vector<CardP>& cards, const String& linked_relation, bool erase_if_no_card) {
+  vector<CardP> other_cards;
+  vector<int> indexes = findRelationLinks(linked_relation);
+  for (int i = 0; i < indexes.size(); ++i) {
+    String& linked_uid = getLinkedUID(indexes[i]);
+    CardP other_card = getUIDCard(cards, linked_uid);
+    if (other_card) other_cards.push_back(other_card);
+    else if (erase_if_no_card) {
+      linked_uid = _("");
+      getLinkedRelation(indexes[i]) = _("");
+    }
+  }
+  return other_cards;
 }
-
-void Card::unlink(const vector<CardP>& unlinked_cards)
-{
-  for (size_t pos = 0; pos < unlinked_cards.size(); ++pos) {
-    CardP unlinked_card = unlinked_cards[pos];
-    unlink(unlinked_card);
-  }
-}
-
-pair<String, String> Card::unlink(CardP& unlinked_card)
-{
-  String old_selected_relation = _("");
-  THIS_LINKED_PAIRS(this_linked_pairs);
-  FOR_EACH(this_linked_pair, this_linked_pairs) {
-    String& this_linked_uid = this_linked_pair.first.get();
-    String& this_linked_relation = this_linked_pair.second.get();
-    if (this_linked_uid == unlinked_card->uid) {
-      old_selected_relation = this_linked_relation;
-      this_linked_uid = _("");
-      this_linked_relation = _("");
-    }
-  }
-  String old_unlinked_relation = _("");
-  OTHER_LINKED_PAIRS(unlinked_pairs, unlinked_card);
-  FOR_EACH(unlinked_pair, unlinked_pairs) {
-    String& unlinked_uid = unlinked_pair.first.get();
-    String& unlinked_relation = unlinked_pair.second.get();
-    if (unlinked_uid == uid) {
-      old_unlinked_relation = unlinked_relation;
-      unlinked_uid = _("");
-      unlinked_relation = _("");
-    }
-  }
-  return make_pair(old_selected_relation, old_unlinked_relation);
-}
-
-void Card::copyLink(const Set& set, String old_uid, String new_uid) {
-  // Find what relation we need to copy
-  String relation_copy = _("");
-  THIS_LINKED_PAIRS(this_linked_pairs);
-  FOR_EACH(this_linked_pair, this_linked_pairs) {
-    String& this_linked_uid = this_linked_pair.first.get();
-    String& this_linked_relation = this_linked_pair.second.get();
-    if (this_linked_uid == old_uid) {
-      relation_copy = this_linked_relation;
-      break;
-    }
-  }
-  // Nothing to copy
-  if (relation_copy.empty()) {
-    return;
-  }
-
-  // Try to copy to a free spot
-  bool written = false;
-  FOR_EACH(this_linked_pair, this_linked_pairs) {
-    String& this_linked_uid = this_linked_pair.first.get();
-    String& this_linked_relation = this_linked_pair.second.get();
-    if (this_linked_uid.empty()) {
-      this_linked_uid = new_uid;
-      this_linked_relation = relation_copy;
-      written = true;
-      break;
-    }
-  }
-  // Try to copy to an erasable spot
-  if (!written) {
-    unordered_set<String> all_existing_uids;
-    FOR_EACH(card, set.cards) {
-      all_existing_uids.insert(card->uid);
-    }
-    FOR_EACH(this_linked_pair, this_linked_pairs) {
-      String& this_linked_uid = this_linked_pair.first.get();
-      String& this_linked_relation = this_linked_pair.second.get();
-      if (all_existing_uids.find(this_linked_uid) == all_existing_uids.end()) {
-        this_linked_uid = new_uid;
-        this_linked_relation = relation_copy;
-        written = true;
-        break;
-      }
-    }
-  }
-  // Notify we couldn't copy
-  if (!written) {
-    queue_message(MESSAGE_WARNING, _ERROR_("not enough free links for copy"));
-  }
-}
-
-void Card::updateLink(String old_uid, String new_uid) {
-  THIS_LINKED_PAIRS(this_linked_pairs);
-  FOR_EACH(this_linked_pair, this_linked_pairs) {
-    String& this_linked_uid = this_linked_pair.first.get();
-    if (this_linked_uid == old_uid) {
-      this_linked_uid = new_uid;
-      return;
-    }
-  }
+vector<CardP> Card::getLinkedRelationCards(const Set& set, const String& linked_relation, bool erase_if_no_card) {
+  return getLinkedRelationCards(set.cards, linked_relation, erase_if_no_card);
 }
 
 vector<pair<CardP, String>> Card::getLinkedCards(const vector<CardP>& cards) {
@@ -283,7 +188,7 @@ vector<pair<CardP, String>> Card::getLinkedCards(const Set& set) {
   return getLinkedCards(set.cards);
 }
 
-CardP Card::getLinkedOtherFace(const vector<CardP>& cards) {
+CardP Card::getLinkedOtherFaceCard(const vector<CardP>& cards) {
   unordered_set<String> faces;
   if (linked_relation_1 == _("Front Face") || linked_relation_1 == _("Back Face")) faces.emplace(linked_card_1);
   if (linked_relation_2 == _("Front Face") || linked_relation_2 == _("Back Face")) faces.emplace(linked_card_2);
@@ -294,39 +199,57 @@ CardP Card::getLinkedOtherFace(const vector<CardP>& cards) {
   }
   return nullptr;
 }
-CardP Card::getLinkedOtherFace(const Set& set) {
-  return getLinkedOtherFace(set.cards);
+CardP Card::getLinkedOtherFaceCard(const Set& set) {
+  return getLinkedOtherFaceCard(set.cards);
 }
 
-vector<CardP> Card::getLinkedCardsFromLink(const vector<CardP>& cards, const String& link, bool erase_if_no_card) {
-  vector<CardP> other_cards;
-  THIS_LINKED_PAIRS(this_linked_pairs);
-  FOR_EACH(this_linked_pair, this_linked_pairs) {
-    String& this_linked_uid = this_linked_pair.first.get();
-    String& this_linked_relation = this_linked_pair.second.get();
-    if (this_linked_relation == link) {
-      CardP other_card = getCardFromUid(cards, this_linked_uid);
-      if (other_card) other_cards.push_back(other_card);
-      else if (erase_if_no_card) {
-        this_linked_relation = _("");
-        this_linked_uid = _("");
-      }
-    }
+void Card::addLink(const Set& set, CardP& linked_card, const String& selected_relation, const String& linked_relation) {
+  unordered_set<String> all_existing_uids;
+  FOR_EACH(card, set.cards) {
+    all_existing_uids.insert(card->uid);
+  }
+
+  int index = findFreeLink(linked_card->uid, all_existing_uids);
+  if (index < 0) {
+    queue_message(MESSAGE_ERROR, _ERROR_1_("not enough free links", identification()));
+    return;
+  }
+  getLinkedUID(index) = linked_card->uid;
+  getLinkedRelation(index) = selected_relation;
+
+  index = linked_card->findFreeLink(uid, all_existing_uids);
+  if (index < 0) {
+    queue_message(MESSAGE_ERROR, _ERROR_1_("not enough free links", linked_card->identification()));
+  }
+  else {
+    linked_card->getLinkedUID(index) = uid;
+    linked_card->getLinkedRelation(index) = linked_relation;
+  }
+}
+
+void Card::removeLink(const CardP& linked_card)
+{
+  int index = findUIDLink(linked_card->uid);
+  if (index >= 0) {
+    getLinkedUID(index) = _("");
+    getLinkedRelation(index) = _("");
+  }
+
+  index = linked_card->findUIDLink(uid);
+  if (index >= 0) {
+    linked_card->getLinkedUID(index) = _("");
+    linked_card->getLinkedRelation(index) = _("");
   }
-  return other_cards;
-}
-vector<CardP> Card::getLinkedCardsFromLink(const Set& set, const String& link, bool erase_if_no_card) {
-  return getLinkedCardsFromLink(set.cards, link, erase_if_no_card);
 }
 
-CardP Card::getCardFromUid(const vector<CardP>& cards, const String& uid) {
+CardP Card::getUIDCard(const vector<CardP>& cards, const String& uid) {
   FOR_EACH(card, cards) {
     if (card->uid == uid) return card;
   }
   return nullptr;
 }
-CardP Card::getCardFromUid(const Set& set, const String& uid) {
-  return getCardFromUid(set.cards, uid);
+CardP Card::getUIDCard(const Set& set, const String& uid) {
+  return getUIDCard(set.cards, uid);
 }
 
 IndexMap<FieldP, ValueP>& Card::extraDataFor(const StyleSheet& stylesheet) {

src/data/card.hpp

@@ -13,6 +13,7 @@
 #include <util/error.hpp>
 #include <data/filter.hpp>
 #include <data/field.hpp> // for Card::value
+#include <unordered_set>
 
 class Game;
 class Dependency;
@@ -23,8 +24,7 @@ DECLARE_POINTER_TYPE(Field);
 DECLARE_POINTER_TYPE(Value);
 DECLARE_POINTER_TYPE(StyleSheet);
 
-#define THIS_LINKED_PAIRS(var)              vector<pair<reference_wrapper<String>, reference_wrapper<String>>> var { make_pair(ref(linked_card_1), ref(linked_relation_1)), make_pair(ref(linked_card_2), ref(linked_relation_2)), make_pair(ref(linked_card_3), ref(linked_relation_3)), make_pair(ref(linked_card_4), ref(linked_relation_4)) }
-#define OTHER_LINKED_PAIRS(var, other_card) vector<pair<reference_wrapper<String>, reference_wrapper<String>>> var { make_pair(ref(other_card->linked_card_1), ref(other_card->linked_relation_1)), make_pair(ref(other_card->linked_card_2), ref(other_card->linked_relation_2)), make_pair(ref(other_card->linked_card_3), ref(other_card->linked_relation_3)), make_pair(ref(other_card->linked_card_4), ref(other_card->linked_relation_4)) }
+#define LINK_PAIRS(var, card) vector<pair<reference_wrapper<String>, reference_wrapper<String>>> var { make_pair(ref(card->linked_card_1), ref(card->linked_relation_1)), make_pair(ref(card->linked_card_2), ref(card->linked_relation_2)), make_pair(ref(card->linked_card_3), ref(card->linked_relation_3)), make_pair(ref(card->linked_card_4), ref(card->linked_relation_4)) }
 
 // ----------------------------------------------------------------------------- : Card
 
@@ -80,27 +80,6 @@ public:
   /// Does any field contains the given query string?
   bool contains(QuickFilterPart const& query) const;
 
-  /// Link or unlink other cards to this card
-  void link(const Set& set, const vector<CardP>& linked_cards, const String& selected_relation, const String& linked_relation);
-  void link(const Set& set, CardP& linked_card, const String& selected_relation, const String& linked_relation);
-  void unlink(const vector<CardP>& linked_cards);
-  pair<String, String> unlink(CardP& unlinked_card); // Returns the relations that were deleted, so we can undo
-
-  void copyLink(const Set& set, String old_uid, String new_uid);
-  void updateLink(String old_uid, String new_uid);
-
-  vector<pair<CardP, String>> getLinkedCards(const vector<CardP>& cards);
-  vector<pair<CardP, String>> getLinkedCards(const Set& set);
-
-  vector<CardP> getLinkedCardsFromLink(const vector<CardP>& cards, const String& link, bool erase_if_no_card);
-  vector<CardP> getLinkedCardsFromLink(const Set& set, const String& link, bool erase_if_no_card);
-
-  CardP getLinkedOtherFace(const vector<CardP>& cards);
-  CardP getLinkedOtherFace(const Set& set);
-
-  static CardP getCardFromUid(const vector<CardP>& cards, const String& uid);
-  static CardP getCardFromUid(const Set& set, const String& uid);
-
   /// Find a value in the data by name and type
   template <typename T> T& value(const String& name) {
     for(IndexMap<FieldP, ValueP>::iterator it = data.begin() ; it != data.end() ; ++it) {
@@ -122,6 +101,46 @@ public:
     }
     throw InternalError(_("Expected a card field with name '")+name+_("'"));
   }
+
+  /// Find the index of a free link slot to write to. Returns -1 if not found.
+  int         findFreeLink (const String&   linked_uid,  const unordered_set<String>& all_existing_uids);
+  vector<int> findFreeLinks(vector<String>& linked_uids, const unordered_set<String>& all_existing_uids);
+  
+  /// Find the index of a link slot that references the linked_uid. Returns -1 if not found.
+  int findUIDLink(const String& linked_uid);
+  /// Find all indexes of link slots that references the linked_relation.
+  vector<int> findRelationLinks(const String& linked_relation);
+
+  /// Get a reference to the linked uid slot.
+  String& getLinkedUID     (int index);
+  /// Get a reference to the linked relation slot.
+  String& getLinkedRelation(int index);
+
+  /// Make all links that point to old_uid point to new_uid instead.
+  void updateLinkedUID(const String& old_uid, const String& new_uid);
+  /// Make all links that have old_relation have new_relation instead. Not needed apparently.
+  //void updateLinkedRelation(const String& old_relation, const String& new_relation);
+
+  /// Get the card with the given uid.
+  static CardP getUIDCard(const vector<CardP>& cards, const String& uid);
+  static CardP getUIDCard(const Set& set,             const String& uid);
+  /// Get all the cards linked to this card with the given relation.
+  vector<CardP> getLinkedRelationCards(const vector<CardP>& cards, const String& linked_relation, bool erase_if_no_card = true);
+  vector<CardP> getLinkedRelationCards(const Set& set,             const String& linked_relation, bool erase_if_no_card = true);
+
+  /// Get all the cards linked to this card.
+  vector<pair<CardP, String>> getLinkedCards(const vector<CardP>& cards);
+  vector<pair<CardP, String>> getLinkedCards(const Set& set);
+
+  /// Get the back face or front face of this card.
+  CardP getLinkedOtherFaceCard(const vector<CardP>& cards);
+  CardP getLinkedOtherFaceCard(const Set& set);
+
+  /// Link a card to this card.
+  void addLink(const Set& set, CardP& linked_card, const String& selected_relation, const String& linked_relation);
+
+  /// Unlink a card from this card.
+  void removeLink(const CardP& linked_card);
 
   DECLARE_REFLECTION();
 };

src/data/game.cpp

@@ -67,6 +67,19 @@ IMPLEMENT_REFLECTION(Game) {
   REFLECT_NO_SCRIPT(auto_replaces);
 }
 
+void Game::add_alt_name_or_warn(const String& alt_name, const String& field_name)
+{
+  String unified_name = unified_form(alt_name);
+  if (card_fields_alt_names.count(unified_name)) {
+    if (card_fields_alt_names[unified_name] != field_name) {
+      queue_message(MESSAGE_WARNING, _("Duplicate alt card field name ' ") + unified_name + _(" ' is both an alt for: ' ") + field_name + _(" ' and ' ") + card_fields_alt_names[unified_name] + _(" '."));
+    }
+  }
+  else {
+    card_fields_alt_names.emplace(unified_name, field_name);
+  }
+}
+
 void Game::validate(Version v) {
   // check that we have at least one card field
   if (card_fields.size() < 1) {
@@ -104,24 +117,13 @@ void Game::validate(Version v) {
   }
   // alternate card field names map
   for (auto it = card_fields.begin(); it != card_fields.end(); ++it) {
-    FieldP field = *it;
-    String unified_name = unified_form(field->name);
-    if (card_fields_alt_names.count(unified_name)) {
-      queue_message(MESSAGE_WARNING, _("Duplicate alternate card field name: ") + unified_name);
-    }
-    else {
-      card_fields_alt_names.emplace(unified_name, field->name);
-    }
-    //String column_name = field->card_list_name.get();
-    //card_fields_alt_names.emplace(unified_form(column_name), field->name);
+    FieldP field = *it;
+    String& field_name = field->name;
+    add_alt_name_or_warn(field_name, field_name);
+    add_alt_name_or_warn(field->card_list_name.default_, field_name);
+    add_alt_name_or_warn(field->card_list_name.get(), field_name);
     for (auto it2 = field->alt_names.begin(); it2 != field->alt_names.end(); ++it2) {
-      unified_name = unified_form(*it2);
-      if (card_fields_alt_names.count(unified_name)) {
-        queue_message(MESSAGE_WARNING, _("Duplicate alternate card field name: ") + unified_name);
-      }
-      else {
-        card_fields_alt_names.emplace(unified_name, field->name);
-      }
+      add_alt_name_or_warn(*it2, field_name);
     }
   }
   // front face/back face card link

src/data/game.hpp

@@ -78,6 +78,7 @@ public:
   bool isMagic() const;
 
 protected:
+  void add_alt_name_or_warn(const String& alt_name, const String& field_name);
   void validate(Version) override;
   
   DECLARE_REFLECTION_OVERRIDE();