continue to simplify the code...

src/dialogxml/widgets/pict.cpp

@@ -1113,7 +1113,6 @@ void cPict::drawCustomTerAnim(short num, rectangle to_rect){
 		return;
 	to_rect.right = to_rect.left + 28;
 	to_rect.bottom = to_rect.top + 36;
-	num += animFrame % 4;
 	rect_draw_some_item(source, source_rect, *inWindow, to_rect);
 }
 

src/fileio/fileio_party.cpp

@@ -91,6 +91,7 @@ bool load_party(fs::path const &file_to_load, cUniverse& univ){
 	}
 	
 	fclose(file_id);
+	spec_scen_g.party_sheet=Texture(); // FIXME: normally we must only reset this sheet if the party is loaded correctly
 	switch(format){
 		case old_mac:
 			return load_party_v1(file_to_load, univ, town_restore, in_scen, maps_there, true);

src/fileio/fileio_scen.cpp

@@ -197,8 +197,6 @@ bool load_scenario_v1(fs::path file_to_load, cScenario& scenario, bool only_head
 	bool file_ok = false;
 	long len;
 	char temp_str[256];
-	legacy::scenario_data_type *temp_scenario = new legacy::scenario_data_type;
-	legacy::scen_item_data_type *item_data = new legacy::scen_item_data_type;
 	// TODO: Convert this (and all the others in this file) to use C++ streams
 	FILE* file_id = fopen(file_to_load.string().c_str(),"rb");
 	if(file_id == nullptr) {
@@ -234,28 +232,30 @@ bool load_scenario_v1(fs::path file_to_load, cScenario& scenario, bool only_head
 	}
 	
 	len = (long) sizeof(legacy::scenario_data_type);
-	if(fread(temp_scenario, len, 1, file_id) < 1) {
+	legacy::scenario_data_type temp_scenario;
+	if(fread(&temp_scenario, len, 1, file_id) < 1) {
 		showError(err_prefix + "Failed to read scenario data.", get_file_error());
 		fclose(file_id);
 		return false;
 	}
-	porting::port_scenario(temp_scenario);
+	porting::port_scenario(&temp_scenario);
 	len = sizeof(legacy::scen_item_data_type); // item data
-	if(fread(item_data, len, 1, file_id) < 1) {
+	legacy::scen_item_data_type item_data;
+	if(fread(&item_data, len, 1, file_id) < 1) {
 		showError(err_prefix + "Failed to read scenario items.", get_file_error());
 		fclose(file_id);
 		return false;
 	}
-	porting::port_item_list(item_data);
+	porting::port_item_list(&item_data);
-	scenario.import_legacy(*temp_scenario);
+	scenario.import_legacy(temp_scenario);
-	scenario.import_legacy(*item_data);
+	scenario.import_legacy(item_data);
 	
 	// TODO: Consider skipping the fread and assignment when len is 0
 	scenario.special_items.resize(50);
 	scenario.journal_strs.resize(50);
 	scenario.spec_strs.resize(100);
 	for(short i = 0; i < 270; i++) {
-		len = (long) (temp_scenario->scen_str_len[i]);
+		len = (long) (temp_scenario.scen_str_len[i]);
 		fread(temp_str, len, 1, file_id);
 		temp_str[len] = 0;
 		
@@ -286,11 +286,11 @@ bool load_scenario_v1(fs::path file_to_load, cScenario& scenario, bool only_head
 	load_spec_graphics_v1(scenario.scen_file);
 	
 	// Now load all the outdoor sectors
-	scenario.outdoors.resize(temp_scenario->out_width, temp_scenario->out_height);
+	scenario.outdoors.resize(temp_scenario.out_width, temp_scenario.out_height);
-	for(int x = 0; x < temp_scenario->out_width; x++) {
+	for(int x = 0; x < temp_scenario.out_width; x++) {
-		for(int y = 0; y < temp_scenario->out_height; y++) {
+		for(int y = 0; y < temp_scenario.out_height; y++) {
 			scenario.outdoors[x][y] = new cOutdoors(scenario);
-			load_outdoors_v1(scenario.scen_file, loc(x,y), *scenario.outdoors[x][y], *temp_scenario);
+			load_outdoors_v1(scenario.scen_file, loc(x,y), *scenario.outdoors[x][y], temp_scenario);
 		}
 	}
 	
@@ -300,12 +300,12 @@ bool load_scenario_v1(fs::path file_to_load, cScenario& scenario, bool only_head
 	// Then load all the towns
 	scenario.towns.resize(scenario.format.num_towns);
 	for(int i = 0; i < scenario.format.num_towns; i++) {
-		switch(temp_scenario->town_size[i]) {
+		switch(temp_scenario.town_size[i]) {
 			case 0: scenario.towns[i] = new cTown(scenario, AREA_LARGE); break;
 			case 1: scenario.towns[i] = new cTown(scenario, AREA_MEDIUM); break;
 			case 2: scenario.towns[i] = new cTown(scenario, AREA_SMALL); break;
 		}
-		load_town_v1(scenario.scen_file, i, *scenario.towns[i], *temp_scenario, shops);
+		load_town_v1(scenario.scen_file, i, *scenario.towns[i], temp_scenario, shops);
 	}
 	// Enable character creation in starting town
 	scenario.towns[scenario.which_town_start]->has_tavern = true;
@@ -375,8 +375,6 @@ bool load_scenario_v1(fs::path file_to_load, cScenario& scenario, bool only_head
 		scenario.shops.push_back(shop);
 	}
 	
-	delete temp_scenario;
-	delete item_data;
 	return true;
 }
 

src/fileio/special_parse.cpp

@@ -114,12 +114,7 @@ static void warn_missing_opcode(unsigned short i) {
 static void init_specials_parse() {
 	opcode.add((*eSpecType::NONE).opcode().c_str(), eSpecType::NONE);
 	// Fill in all the opcodes and check for missing types.
-	// There's really no need to check all the way to the max of the underlying type.
+	for(unsigned short i = 1; i <= int(eSpecType::MAX_SPEC_TYPE); i++) {
-	// It's unlikely we'd go above 255, so unsigned char would be fine, but just in case,
-	// let's use unsigned short.
-	// Could change the actual enum's underlying type instead though?
-	using underlying = unsigned short;
-	for(underlying i = 1; i < std::numeric_limits<underlying>::max(); i++) {
 		eSpecType check = (eSpecType) i;
 		eSpecCat category = (*check).cat;
 		if(category == eSpecCat::INVALID) continue;

src/gfx/gfxsheets.cpp

@@ -100,9 +100,9 @@ void cCustomGraphics::copy_graphic(pic_num_t dest, pic_num_t src, size_t numSlot
 		std::tie(from_sheet,from_rect) = find_graphic(src + i);
 		std::tie(to_sheet,to_rect) = find_graphic(dest + i, true);
 		if(to_sheet.texture != last_src) {
-			if(last_src) *last_src=sf::Texture(temp.getTexture());
+			if(last_src) *last_src=sf::Texture(temp.getTexture()); // save the old picture
 			last_src = std::const_pointer_cast<sf::Texture>(to_sheet.texture);
-			temp.create(to_sheet->getSize().x, to_sheet->getSize().y);
+			temp.create(to_sheet->getSize().x, to_sheet->getSize().y); // recreate a new picture from the new sheet
 			rect_draw_some_item(to_sheet, rectangle(to_sheet), temp, rectangle(*to_sheet.texture));
 		}
 		rect_draw_some_item(from_sheet, from_rect, temp,

src/gfx/qdpict.mac.cpp

@@ -467,11 +467,11 @@ bool tryLoadPictFromResourceFile(fs::path& gpath, sf::Image& graphics_store) {
 	// TODO: There's no way around it; I'll have to read resource files for this section.
 	FSRef file;
 	ResFileRefNum custRef;
-	OSErr err = FSPathMakeRef((const UInt8*)gpath.c_str(), &file, nullptr);
+	FSPathMakeRef((const UInt8*)gpath.c_str(), &file, nullptr);
-	err = FSOpenResourceFile(&file, 0, nullptr, fsRdPerm, &custRef);
+	OSErr err = FSOpenResourceFile(&file, 0, nullptr, fsRdPerm, &custRef);
 	if(err != noErr) { // TODO: Is the error that would be returned if the resources were stored in the resource fork?
 		HFSUniStr255 rsrc;
-		err = FSGetResourceForkName(&rsrc);
+		FSGetResourceForkName(&rsrc);
 		err = FSOpenResourceFile(&file, rsrc.length, rsrc.unicode, fsRdPerm, &custRef);
 		if(err != noErr) {
 			showError("An old-style .meg graphics file was found, but neither data nor resource fork could be read.",noGraphics);

src/scenario/special.cpp

@@ -688,12 +688,7 @@ static eSpecCat getNodeCategory(eSpecType node) {
 
 static std::map<eSpecType, node_properties_t> loadProps() {
 	std::map<eSpecType, node_properties_t> allNodeProps;
-	// There's really no need to check all the way to the max of the underlying type.
+	for(unsigned short i = 0; i <= int(eSpecType::MAX_SPEC_TYPE); i++) {
-	// It's unlikely we'd go above 255, so unsigned char would be fine, but just in case,
-	// let's use unsigned short.
-	// Could change the actual enum's underlying type instead though?
-	using underlying = unsigned short;//std::underlying_type<eSpecType>::type;
-	for(underlying i = 0; i < std::numeric_limits<underlying>::max(); i++) {
 		eSpecType check = (eSpecType) i;
 		eSpecCat category = getNodeCategory(check);
 		if(category == eSpecCat::INVALID) continue;

src/scenario/special.hpp

@@ -66,6 +66,8 @@ enum class eSpecType {
 	RECT_SWAP_TER = 215, RECT_TRANS_TER = 216, RECT_LOCK = 217, RECT_UNLOCK = 218,
 	
 	OUT_MAKE_WANDER = 225, OUT_FORCE_TOWN = 226, OUT_PLACE_ENCOUNTER = 227, OUT_MOVE_PARTY = 228,
+	
+	MAX_SPEC_TYPE=OUT_MOVE_PARTY
 };
 
 class cSpecial {

src/universe/universe.cpp

@@ -1061,6 +1061,13 @@ struct cCustomUpdateState {
 	
 	std::set<cItem const *> seenItem;
 	std::set<cMonster const *> seenMonster;
+	void insert_missile_pict(miss_num_t &pict_id) {
+		if(pict_id >= 10000) {
+			for(int i = 0; i < 4; i++)
+				graphics.insert(pict_id - 10000 + i);
+		} else if(pict_id >= 1000)
+			missiles[pict_id - 1000].insert(&pict_id);
+	}
 	pic_num_t add_graphic(pic_num_t pic, ePicType type);
 	void check_monst(cUniverse &univers, cMonster & monst);
 	void check_item(cUniverse &univers, cItem& item);
@@ -1117,20 +1124,10 @@ void cCustomUpdateState::check_monst(cUniverse &univers, cMonster & monst) {
 	for(auto& abil : monst.abil) {
 		switch(getMonstAbilCategory(abil.first)) {
 			case eMonstAbilCat::MISSILE:
-				if(abil.second.missile.pic >= 10000) {
+				insert_missile_pict(abil.second.missile.pic);
-					for(int i = 0; i < 4; i++)
-						graphics.insert(abil.second.missile.pic - 10000 + i);
-				} else if(abil.second.missile.pic >= 1000) {
-					missiles[abil.second.missile.pic - 1000].insert(&abil.second.missile.pic);
-				}
 				break;
 			case eMonstAbilCat::GENERAL:
-				if(abil.second.gen.pic >= 10000) {
+				insert_missile_pict(abil.second.gen.pic);
-					for(int i = 0; i < 4; i++)
-						graphics.insert(abil.second.gen.pic - 10000 + i);
-				} else if(abil.second.gen.pic >= 1000) {
-					missiles[abil.second.gen.pic - 1000].insert(&abil.second.gen.pic);
-				}
 				break;
 			case eMonstAbilCat::SUMMON:
 				if(abil.second.summon.type == eMonstSummon::TYPE)
@@ -1158,13 +1155,8 @@ void cCustomUpdateState::check_item(cUniverse &universe, cItem& item) {
 			check_monst(universe, universe.party.summons[monst - 10000]);
 		else check_monst(universe, universe.scenario.scen_monsters[monst]);
 	}
-	if(item.variety == eItemType::ARROW || item.variety == eItemType::BOLTS || item.variety == eItemType::MISSILE_NO_AMMO || item.variety == eItemType::THROWN_MISSILE) {
+	if(item.variety == eItemType::ARROW || item.variety == eItemType::BOLTS || item.variety == eItemType::MISSILE_NO_AMMO || item.variety == eItemType::THROWN_MISSILE)
-		if(item.missile >= 10000)
+		insert_missile_pict(item.missile);
-			for(int i = 0; i < 4; i++)
-				graphics.insert(item.missile - 10000 + i);
-		else if(item.missile >= 1000)
-			missiles[item.missile - 1000].insert(&item.missile);
-	}
 }
 
 void cUniverse::exportGraphics() {
@@ -1234,29 +1226,29 @@ void cUniverse::exportSummons() {
 	for(int i = 0; i < 6; i++) {
 		if(party[i].main_status == eMainStatus::ABSENT)
 			continue;
-		for(size_t j = 0; j < party[i].items.size(); j++) {
+		for(auto &item : party[i].items) {
-			if(party[i].items[j].variety == eItemType::NO_ITEM) continue;
+			if(item.variety == eItemType::NO_ITEM) continue;
-			if(party[i].items[j].ability == eItemAbil::SUMMONING || party[i].items[j].ability == eItemAbil::MASS_SUMMONING) {
+			if(item.ability == eItemAbil::SUMMONING || item.ability == eItemAbil::MASS_SUMMONING) {
-				mon_num_t monst = party[i].items[j].abil_data[1];
+				mon_num_t monst = item.abil_data[1];
 				if(monst >= 10000)
 					used_monsters.insert(monst - 10000);
 				else {
 					need_monsters.insert(monst);
-					update_items[monst].insert(&party[i].items[j]);
+					update_items[monst].insert(&item);
 				}
 			}
 		}
 	}
-	for(size_t i = 0; i < party.stored_items.size(); i++) {
+	for(auto &items : party.stored_items) {
-		for(size_t j = 0; j < party.stored_items[i].size(); j++) {
+		for(auto &item : items) {
-			if(party.stored_items[i][j].variety == eItemType::NO_ITEM) continue;
+			if(item.variety == eItemType::NO_ITEM) continue;
-			if(party.stored_items[i][j].ability == eItemAbil::SUMMONING||party.stored_items[i][j].ability == eItemAbil::MASS_SUMMONING) {
+			if(item.ability == eItemAbil::SUMMONING||item.ability == eItemAbil::MASS_SUMMONING) {
-				mon_num_t monst = party.stored_items[i][j].abil_data[1];
+				mon_num_t monst = item.abil_data[1];
 				if(monst >= 10000)
 					used_monsters.insert(monst - 10000);
 				else {
 					need_monsters.insert(monst);
-					update_items[monst].insert(&party.stored_items[i][j]);
+					update_items[monst].insert(&item);
 				}
 			}
 		}