Code review comment for lp:~widelands-dev/widelands/find_attack_soldiers

Revision history for this message
Martin Schmidt (martinschmidt) wrote :

Your patch was really useful, I just changed the sorting to stl and that's it. I also tried a short game and it really makes a difference seeing soldiers coming from all military builings in range to attack instead of weaking one building totally.
Here is the patch:

=== modified file 'src/logic/player.cc'
--- src/logic/player.cc 2014-12-20 22:03:47 +0000
+++ src/logic/player.cc 2015-02-16 14:32:33 +0000
@@ -100,31 +100,26 @@
  int priority; // (strength + 1) * health - position in building.
 };

+/**
+* Find the strongest players with highest priority
+*/
+bool compare_soldiers(const SoldierPriority& a, const SoldierPriority& b) {
+ return a.priority > b.priority;
+}
+
 void sort_soldiers(std::vector<SoldierPriority>* soldiers,
- uint32_t max = std::numeric_limits<uint32_t>::max()) {
- std::vector<SoldierPriority> temp;
- SoldierPriority soldier;
- uint32_t left = soldiers->size();
-
- while (left) {
- int32_t maxlevel = -1;
- uint32_t pos = 0;
- uint32_t maxpos = 0;
- for (const SoldierPriority& soldier_priority : *soldiers) {
- const int32_t level = soldier_priority.priority;
- if (level > maxlevel) {
- maxlevel = level;
- maxpos = pos;
- soldier.soldier = soldier_priority.soldier;
- soldier.priority = level;
- }
- pos++;
- }
- temp.push_back(soldier);
- soldiers->erase(soldiers->begin() + maxpos);
- left--;
+ uint32_t max = std::numeric_limits<uint32_t>::max()) {
+ if (max >= soldiers->size()) {
+ return;
  }
- soldiers->insert(soldiers->end(), temp.begin(), temp.begin() + std::min<int>(max, temp.size()));
+
+ std::stable_sort(soldiers->begin(), soldiers->end(), compare_soldiers);
+ // Can an unstable sorting algorithm cause desyncts or are the implementations
+ // of std algorithms guaranteed to return the same (possibly non-stable)
+ // sorting? If yes, partial_sort may be favourable
+ // std::partial_sort(soldiers->begin(), soldiers->begin() + max,
+ // soldiers->end(), compare_soldiers);
+ soldiers->resize(max);
 }

 }

« Back to merge proposal