Cleanup 3rd set of outstanding PRs (#189)

Too few trade goods causes infinite loop
* Pull in the work from PR No trade items bugfix #54
* Avoid an infinite loop when we have less than 4 trade items.

Clean up food item cost display in rules
* Pull in work from PR Rules say no fractional food #120
* Remove an extraneous comma
* Add text saying partial food is lost.
* Update rules snapshots with changes.

Update rules to explain order of maintenance costs
* Pulled in changes from PR Added clarification on how maintenance fee is paid #111
* Added section to rules showing order of maintenance payments.
* Updated snapshots to reflect changes.

Fix longstanding economy bug
* Pulled in changes from PR Update economy.cpp #8
* Economic improvement never counted users selling trade goods to a city/town

Fix logical bug in world generation code
* Pulls in changes from PR Fix using assignation instead of compare #53
* All games except neworigins and standard had this bug - it had been previously fixed (partially) in those 2 codebases, but added parens to make sure comparison was correct.

Small code quality cleanups on delete[] vs delete
* Pulls changes for PR Use of non-array delete #16
* While these didn't cause any errors/warnings during compile they are technically more correct code.

Updated CREDITS file with some names that I was able to find that weren't already recorded.

CREDITS

@@ -13,14 +13,12 @@ Stefan Morrell
     The patch for Apprentices/revamping Lacandon Conquest
 Karl Castle
     Implementation of alternate army rout methods.
-    Patch for bug with losing items other than prepared items when in game
-      with USE_PREPARED_COMMAND
+    Patch for bug with losing items other than prepared items when in game with USE_PREPARED_COMMAND
 Jon Bagshaw
     Working with Stefan on Lacandon Conquest
 Tom Alsen (aka Tzarg)
     Implementation of the FIND ALL order, and a new combat system
-    Implementation of the SPOILS code which allows saner battle spoils
-      selection.
+    Implementation of the SPOILS code which allows saner battle spoils selection.
     Implmented a GIVE ALL <category> option.
     Multiple other code enhancements.
 Ben Lloyd
@@ -32,25 +30,21 @@ Andrew Zhilenko
 Terho Uotila
     A couple of ideas and items
 Paul Mehrer
-    Submitted patch to allow limiting the number of mages which can study
-      in any give building type.
+    Submitted patch to allow limiting the number of mages which can study in any give building type.
 Stephen Baillie
-    Submitted a patch to allow units passing through a region (but not
-      stopping) to generate a partial, slightly innacurate hex report.
-    Pointed out bugs in the easier underworld code, the improved
-      farsight code and other places.
+    Submitted a patch to allow units passing through a region (but not stopping) to generate a partial,
+        slightly innacurate hex report.
+    Pointed out bugs in the easier underworld code, the improved farsight code and other places.
     Implemented multiple taxation options controlling who could tax.
     Implemented skill improvement through practice code.
-    Changed MOVE and SAIL to queue moves that failed because of lack
-      of movement points, so that @TURN caravans can cope with weather.
+    Changed MOVE and SAIL to queue moves that failed because of lack of movement points, so that
+        @TURN caravans can cope with weather.
     Cleaned up TURN processing significantly.
-    Added code to generate icosahedral (ie, approximately spherical)
-      levels.
+    Added code to generate icosahedral (ie, approximately spherical) levels.
     Various small changes and fixes
 Nick Hoggard
     Implementation of the EXCHANGE order.
-    Implementation of the TURN order which allows pre-doing turn orders as
-      well as cyclical orders.
+    Implementation of the TURN order which allows pre-doing turn orders as well as cyclical orders.
 Peter Rayner
     Patch for 'EVICT' command.
 Vitauts Stochka
@@ -63,21 +57,34 @@ Jan Rietema
 Michael Scalet
     Implemented BANKs, Banking skill and BANK order.
 Sergey Budnevitch
-    Fixed integer overflow bug which allowed creating items via bogus
-      give orders.
+    Fixed integer overflow bug which allowed creating items via bogus give orders.
 Anthony Briggs
     Coastal fish patch
     New Gamemaster guide
     Smoke testing script
-	Added a patch to limit tact-5 leaders based on points spent in war.
-	The Fracas variant
-	Different bullets for units based on your attitude to them
-	The NOSTUDY, NOTEACH and NOEXP flags for skills.
+    Added a patch to limit tact-5 leaders based on points spent in war.
+    The Fracas variant
+    Different bullets for units based on your attitude to them
+    The NOSTUDY, NOTEACH and NOEXP flags for skills.
 Joseph Traub
     Changes, updates, and merging a few disparate branches
     Maintaining and extending the 4.0.X code.
-	Implemented quartermasters and TRANSPORT/DISTRIBUTE
+    Implemented quartermasters and TRANSPORT/DISTRIBUTE
 Peter Christie
-	Added coracles (a boat smaller than a longboat)
+    Added coracles (a boat smaller than a longboat)
 Bradley "Starstrike"
-	Updated the editing code
+    Updated the editing code
+Valdis Zobēla
+    Many updates and patches to the New Origins sourcecode over time.
+Artem Trytiak
+    Maintenance, ownership and improvements for the entire life of the New Origins code.
+Edward Brekelbaum
+    Various bugfixes.
+    Maintainer of the True Atlanteans client.
+David Lozano Ruiz
+    Various bug fixes.
+Henrik Kurelid
+    Various bug fixes.
+Enno Rehling
+    Some bug fixes.
+    Maintainer of the Eressea offshoot of Atlantis

astring.cpp

@@ -88,7 +88,7 @@ AString::AString(const std::string & s) {
 
 AString::~AString()
 {
-	if (str) delete str;
+	if (str) delete[] str;
 	str = NULL;
 }
 
@@ -102,7 +102,7 @@ AString::AString(const AString &s)
 AString & AString::operator=(const AString &s)
 {
 	len = s.len;
-	if (str) delete str;
+	if (str) delete[] str;
 	str = new char[len + 1];
 	strcpy(str,s.str);
 	return *this;
@@ -112,7 +112,7 @@ AString & AString::operator=(const char *c)
 {
 	len = 0;
 	if (c) len = strlen(c);
-	if (str) delete str;
+	if (str) delete[] str;
 	str = new char[len + 1];
 	if (c) strcpy(str,c);
 	return *this;
@@ -184,7 +184,7 @@ AString &AString::operator+=(const AString &s)
 	for (int j=0; j<s.len+1; j++) {
 		temp[i++] = s.str[j];
 	}
-	delete str;
+	delete[] str;
 	str = temp;
 	len = len + s.len;
 	return *this;
@@ -227,7 +227,7 @@ AString *AString::gettoken()
 			place++;
 		} else {
 			/* Unmatched "" return 0 */
-			delete str;
+			delete[] str;
 			str = new char[1];
 			len = 0;
 			str[0] = '\0';
@@ -241,7 +241,7 @@ AString *AString::gettoken()
 	}
 	buf[place2] = '\0';
 	if (place == len || str[place] == ';') {
-		delete str;
+		delete[] str;
 		str = new char[1];
 		len = 0;
 		str[0] = '\0';
@@ -256,7 +256,7 @@ AString *AString::gettoken()
 	}
 	buf2[place2] = '\0';
 	len = newlen;
-	delete str;
+	delete[] str;
 	str = buf2;
 	return new AString(buf);
 }

basic/map.cpp

@@ -498,7 +498,7 @@ void ARegionList::MakeLand(ARegionArray *pRegs, int percentOcean,
 			}
 			for (int i=0; i<sz; i++) {
 				int dir = getrandom(NDIRS);
-				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir = NDIRS-1))
+				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir == (NDIRS-1)))
 					&& (getrandom(4) < 3)) continue;
 				if ((reg->yloc > (yband+yoff)*2) && ((dir < 5) && (dir > 1))
 					&& (getrandom(4) < 3)) continue;				

economy.cpp

@@ -655,6 +655,10 @@ void ARegion::SetupCityMarket()
 		supply[i] = 0;
 		num--;
 	}
+
+	// If we don't have at least 4 trade items don't set up trade markets
+	if (numtrade < 4) return;
+
 	/* Set up the trade items */
 	int buy1 = getrandom(numtrade);
 	int buy2 = getrandom(numtrade);
@@ -668,8 +672,7 @@ void ARegion::SetupCityMarket()
 	buy1 = getrandom(numtrade);
 	while (buy1 == buy2) buy2 = getrandom(numtrade);
 	while (sell1 == buy1 || sell1 == buy2) sell1 = getrandom(numtrade);
-	while (sell2 == sell1 || sell2 == buy2 || sell2 == buy1)
-		sell2 = getrandom(numtrade);
+	while (sell2 == sell1 || sell2 == buy2 || sell2 == buy1) sell2 = getrandom(numtrade);
 
 	for (int i=0; i<NITEMS; i++) {
 		if (ItemDefs[i].flags & ItemType::DISABLED) continue;
@@ -1192,27 +1195,25 @@ int ARegion::TownGrowth()
 		int tot = 0;
 		for (const auto& m : markets) {
 			if (Population() > m->minpop) {
-				if (ItemDefs[m->item].type & IT_TRADE) {
-					if (m->type == M_BUY) {
+				if (m->type == M_BUY) {
+					if (ItemDefs[m->item].type & IT_TRADE) {
 						amt += 5 * m->activity;
 						tot += 5 * m->maxamt;
 						/* regional economic improvement */
 						improvement += 3 * amt;
 					}
-				} else {
-					if (m->type == M_SELL) {
-						// Only food items except fish are mandatory
-						// for town growth - other items can
-						// be used in replacement
-						if (ItemDefs[m->item].type & IT_FOOD) {
-							amt += 2 * m->activity;
-						} else amt += m->activity;
-						if ((ItemDefs[m->item].type & IT_FOOD)
-							&& (m->item != I_FISH))	tot += 2 * m->maxamt;
-						if (ItemDefs[m->item].type & IT_TRADE) {
-							/* regional economic improvement */
-							improvement += 3 * amt;
-						}
+				} else { // m->type == M_SELL
+					// Only food items except fish are mandatory
+					// for town growth - other items can
+					// be used in replacement
+					if (ItemDefs[m->item].type & IT_FOOD) {
+						amt += 2 * m->activity;
+					} else amt += m->activity;
+					if ((ItemDefs[m->item].type & IT_FOOD) && (m->item != I_FISH))
+						tot += 2 * m->maxamt;
+					if (ItemDefs[m->item].type & IT_TRADE) {
+						/* regional economic improvement */
+						improvement += 3 * amt;
 					}
 				}
 			}

fracas/map.cpp

@@ -498,7 +498,7 @@ void ARegionList::MakeLand(ARegionArray *pRegs, int percentOcean,
 			}
 			for (int i=0; i<sz; i++) {
 				int dir = getrandom(NDIRS);
-				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir = NDIRS-1))
+				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir == (NDIRS-1)))
 					&& (getrandom(4) < 3)) continue;
 				if ((reg->yloc > (yband+yoff)*2) && ((dir < 5) && (dir > 1))
 					&& (getrandom(4) < 3)) continue;				

game.cpp

@@ -69,7 +69,7 @@ Game::Game()
 
 Game::~Game()
 {
-	delete ppUnits;
+	delete[] ppUnits;
 	delete events;
 	ppUnits = 0;
 	maxppunits = 0;
@@ -1323,7 +1323,7 @@ void Game::SetupUnitSeq()
 
 void Game::SetupUnitNums()
 {
-	if (ppUnits) delete ppUnits;
+	if (ppUnits) delete[] ppUnits;
 
 	SetupUnitSeq();
 
@@ -1383,7 +1383,7 @@ Unit *Game::GetNewUnit(Faction *fac, int an)
 		Unit **temp = new Unit*[maxppunits+10000];
 		memcpy(temp, ppUnits, maxppunits*sizeof(Unit *));
 		maxppunits += 10000;
-		delete ppUnits;
+		delete[] ppUnits;
 		ppUnits = temp;
 	}
 

genrules.cpp

@@ -2085,22 +2085,29 @@ int Game::GenRules(const AString &rules, const AString &css, const AString &intr
 		}
 	}
 	f << ".";
-	if (Globals->FOOD_ITEMS_EXIST) {
+	if (Globals->FOOD_ITEMS_EXIST && Globals->UPKEEP_FOOD_VALUE > 0) {
 		f << " Units may substitute one unit of ";
-		int j = 0;
+		bool first = true;
 		int last = -1;
 		for (int i = 0; i < NITEMS; i++) {
 			if (ItemDefs[i].flags & ItemType::DISABLED) continue;
 			if (!(ItemDefs[i].type & IT_FOOD)) continue;
 			if (last != -1) {
-				if (j > 0) f << ", ";
+				if (!first) f << ", ";
 				f << ItemDefs[last].names;
+				first = false;
 			}
 			last = i;
-			j++;
 		}
-		if (j > 0) f << " or ";
-		f << ItemDefs[last].names << " for each " << Globals->UPKEEP_FOOD_VALUE << " silver of maintenance owed. ";
+		if (!first) f << " or ";
+		f << ItemDefs[last].names << " for each " << Globals->UPKEEP_FOOD_VALUE << " silver ";
+		bool evenly_divisible = Globals->MAINTENANCE_COST % Globals->UPKEEP_FOOD_VALUE == 0;
+		if (!evenly_divisible) f << "(or fraction thereof) ";
+		f << "of maintenance owed. ";
+		if (!evenly_divisible) {
+			f << "Food value for a fractional maintenance cost still consumes the entire unit of food. ";
+		}
+
 		if (Globals->UPKEEP_MINIMUM_FOOD > 0) {
 			f << "A unit must be given at least " << Globals->UPKEEP_MINIMUM_FOOD
 			  << " maintenance per man in the form of food. ";
@@ -2114,6 +2121,28 @@ int Game::GenRules(const AString &rules, const AString &css, const AString &intr
 		  << "them and using the money is more economical than using them for maintenance.";
 	};
 	f << '\n' << enclose("p", false);
+	f << enclose("p", true);
+	f << "Maintenance costs are paid in the following order:";
+	f << enclose("ol", true);
+	if (Globals->FOOD_ITEMS_EXIST && Globals->UPKEEP_FOOD_VALUE > 0) {
+		f << enclose("li", true) << "Food items the unit owns if the unit is set " << url("#consume", "CONSUME UNIT")
+		  << "or " << url("#consume", "CONSUME FACTION") << "\n" << enclose("li", false);
+		f << enclose("li", true) << "Food items from faction units in the same region if the unit is set "
+		  << url("#consume", "CONSUME FACTION") << "\n" << enclose("li", false);
+	}
+	f << enclose("li", true) << "Silver in the unit's possession\n" << enclose("li", false);
+	f << enclose("li", true) << "Silver from other faction units in the same region.\n" << enclose("li", false);
+	if (Globals->FOOD_ITEMS_EXIST && Globals->UPKEEP_FOOD_VALUE > 0) {
+		f << enclose("li", true) << "Food items in the unit's possession.\n" << enclose("li", false);
+		f << enclose("li", true) << "Food items from faction units in the same region.\n" << enclose("li", false);
+	}
+	f << enclose("li", true) << "Unclaimed silver.\n" << enclose("li", false);
+	f << enclose("li", true) << "Silver from allied units in the same region.\n" << enclose("li", false);
+	if (Globals->FOOD_ITEMS_EXIST && Globals->UPKEEP_FOOD_VALUE > 0) {
+		f << enclose("li", true) << "Food items from allied units in the same region.\n" << enclose("li", false);
+	}
+	f << enclose("ol", false);
+	f << enclose("p", false);
 
 	f << anchor("economy_recruiting") << '\n';
 	f << enclose("h3", true) << "Recruiting:\n" << enclose("h3", false);

havilah/map.cpp

@@ -506,7 +506,7 @@ void ARegionList::MakeLand(ARegionArray *pRegs, int percentOcean,
 			}
 			for (int i=0; i<sz; i++) {
 				int dir = getrandom(NDIRS);
-				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir = NDIRS-1))
+				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir == (NDIRS-1)))
 					&& (getrandom(4) < 3)) continue;
 				if ((reg->yloc > (yband+yoff)*2) && ((dir < 5) && (dir > 1))
 					&& (getrandom(4) < 3)) continue;				

kingdoms/map.cpp

@@ -643,7 +643,7 @@ void ARegionList::MakeLand(ARegionArray *pRegs, int percentOcean,
 				}
 				for (int i=0; i<sz; i++) {
 					int dir = getrandom(NDIRS);
-					if ((reg->yloc < yoff*2) && ((dir < 2) || (dir = NDIRS-1))
+					if ((reg->yloc < yoff*2) && ((dir < 2) || (dir == (NDIRS-1)))
 						&& (getrandom(4) < 3)) continue;
 					if ((reg->yloc > (yband+yoff)*2) && ((dir < 5) && (dir > 1))
 						&& (getrandom(4) < 3)) continue;				

neworigins/map.cpp

@@ -2162,7 +2162,7 @@ void ARegionList::MakeLand(ARegionArray *pRegs, int percentOcean,
 			}
 			for (int i=0; i<sz; i++) {
 				int dir = getrandom(NDIRS);
-				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir == NDIRS-1))
+				if ((reg->yloc < yoff*2) && ((dir < 2) || (dir == (NDIRS-1)))
 					&& (getrandom(4) < 3)) continue;
 				if ((reg->yloc > (yband+yoff)*2) && ((dir < 5) && (dir > 1))
 					&& (getrandom(4) < 3)) continue;