all: use the function get_item which checks bounds...

src/fileio/fileio_scen.cpp

@@ -342,7 +342,7 @@ bool load_scenario_v1(fs::path file_to_load, cScenario& scenario, bool only_head
 		else if(info.type == eShopItemType::ITEM) {
 			bool is_food_shop = true;
 			for(int i = info.first; i < info.first + info.count && i < scenario.scen_items.size(); i++) {
-				if(scenario.scen_items[i].variety != eItemType::FOOD)
+				if(scenario.get_item(i).variety != eItemType::FOOD)
 					is_food_shop = false;
 			}
 			if(is_food_shop)
@@ -1136,9 +1136,9 @@ void readItemsFromXml(ticpp::Document&& data, cScenario& scenario) {
 			throw xBadNode(type, elem->Row(), elem->Column(), fname);
 		int which_item;
 		elem->GetAttribute("id", &which_item);
-		if(which_item >= scenario.scen_items.size())
+		if(which_item >= scenario.scen_items.size() && which_item<5000) // checkme: what is a reasonnable maximum for the item
 			scenario.scen_items.resize(which_item + 1);
-		cItem& the_item = scenario.scen_items[which_item];
+		cItem& the_item = scenario.get_item(which_item);
 		the_item = cItem();
 		std::set<std::string> reqs = {"variety", "level", "pic", "value", "weight", "name", "full-name"};
 		Iterator<Element> item;

src/game/boe.actions.cpp

@@ -1941,10 +1941,11 @@ bool handle_keystroke(const sf::Event& event){
 		case 'I':
 			if(univ.debug_mode) {
 				int i = get_num_response(0, univ.scenario.scen_items.size()-1, "Which item?");
-				int j = univ.scenario.scen_items[i].ident;
+				cItem &item = univ.get_item(i);
-				univ.scenario.scen_items[i].ident = true;
+				bool saveIdent = item.ident;
-				univ.party.give_item(univ.scenario.scen_items[i], true);
+				item.ident = true;
-				univ.scenario.scen_items[i].ident = j;
+				univ.party.give_item(item, true);
+				item.ident = saveIdent;
 				print_buf();
 				put_item_screen(stat_window);
 				put_pc_screen(); // In case the item was food or gold

src/game/boe.items.cpp

@@ -40,12 +40,10 @@ extern cUniverse univ;
 short selected;
 
 bool GTP(short item_num) {
-	cItem item = univ.scenario.get_stored_item(item_num);
+	return univ.party.give_item(univ.get_item(item_num),true);
-	return univ.party.give_item(item,true);
 }
 bool silent_GTP(short item_num) {
-	cItem item = univ.scenario.get_stored_item(item_num);
+	return univ.party.give_item(univ.get_item(item_num),false);
-	return univ.party.give_item(item,false);
 }
 void give_gold(short amount,bool print_result) {
 	if(amount < 0) return;
@@ -684,10 +682,8 @@ void place_glands(location where,mon_num_t m_type) {
 		monst = univ.party.summons[m_type - 10000];
 	else monst = univ.scenario.scen_monsters[m_type];
 	
-	if((monst.corpse_item >= 0) && (monst.corpse_item < 400) && (get_ran(1,1,100) < monst.corpse_item_chance)) {
+	if(monst.corpse_item >= 0 && monst.corpse_item < univ.scenario.scen_items.size() && get_ran(1,1,100) < monst.corpse_item_chance)
-		store_i = univ.scenario.get_stored_item(monst.corpse_item);
+		place_item(univ.get_item(monst.corpse_item),where);
-		place_item(store_i,where);
-	}
 }
 
 void reset_item_max() {
@@ -705,7 +701,6 @@ short item_val(cItem item) {
 //short mode;  // 0 - normal, 1 - force
 void place_treasure(location where,short level,short loot,short mode) {
 	
-	cItem new_item;
 	short amt,r1;
 	// Make these static const because they are never changed.
 	// Saves them being initialized every time the function is called.
@@ -748,8 +743,9 @@ void place_treasure(location where,short level,short loot,short mode) {
 	if(univ.party.get_level() <= 60 && amt > 2)
 		amt += 2;
 	
+	cItem new_item;
 	if(amt > 3) {
-		new_item = univ.scenario.get_stored_item(0);
+		new_item = univ.scenario.get_item(0);
 		new_item.item_level = amt;
 		r1 = get_ran(1,1,9);
 		if(((loot > 1) && (r1 < 7)) || ((loot == 1) && (r1 < 5)) || (mode == 1)

src/game/boe.specials.cpp

@@ -2217,7 +2217,7 @@ void general_spec(const runtime_state& ctx) {
 			check_mess = true;
 			if(spec.ex1a < 0 || spec.ex1a >= univ.scenario.scen_items.size())
 				break;
-			if(!univ.party.forced_give(univ.scenario.scen_items[spec.ex1a],eItemAbil::NONE) && spec.ex1b >= 0)
+			if(!univ.party.forced_give(univ.get_item(spec.ex1a),eItemAbil::NONE) && spec.ex1b >= 0)
 				ctx.next_spec = spec.ex1b;
 			break;
 		case eSpecType::BUY_ITEMS_OF_TYPE:
@@ -2401,10 +2401,10 @@ void general_spec(const runtime_state& ctx) {
 		case eSpecType::APPEND_ITEM:
 			if(spec.pic) univ.get_buf() += ' ';
 			if(spec.ex1b == 1)
-				univ.get_buf() += univ.scenario.scen_items[spec.ex1a].full_name;
+				univ.get_buf() += univ.get_item(spec.ex1a).full_name;
 			else if(spec.ex1b == 2)
-				univ.get_buf() += get_item_interesting_string(univ.scenario.scen_items[spec.ex1a]);
+				univ.get_buf() += get_item_interesting_string(univ.get_item(spec.ex1a));
-			else univ.get_buf() += univ.scenario.scen_items[spec.ex1a].name;
+			else univ.get_buf() += univ.get_item(spec.ex1a).name;
 			break;
 		case eSpecType::APPEND_TER:
 			if(spec.pic) univ.get_buf() += ' ';
@@ -2500,7 +2500,7 @@ void oneshot_spec(const runtime_state& ctx) {
 	switch(cur_node.type) {
 		case eSpecType::ONCE_GIVE_ITEM:
 			if(spec.ex1a >= 0 && spec.ex1a < univ.scenario.scen_items.size() &&
-					!univ.party.forced_give(univ.scenario.scen_items[spec.ex1a],eItemAbil::NONE)) {
+					!univ.party.forced_give(univ.get_item(spec.ex1a),eItemAbil::NONE)) {
 				set_sd = false;
 				if( spec.ex2b >= 0)
 					ctx.next_spec = spec.ex2b;
@@ -2574,7 +2574,7 @@ void oneshot_spec(const runtime_state& ctx) {
 			dlg_res = custom_choice_dialog(strs, spec.pic, ePicType(spec.pictype), buttons);
 			if(dlg_res == 1) {set_sd = false; ctx.next_spec = -1;}
 			else {
-				store_i = univ.scenario.get_stored_item(spec.ex1a);
+				store_i = univ.get_item(spec.ex1a);
 				if((spec.ex1a >= 0) && (!univ.party.give_item(store_i,true))) {
 					set_sd = false; ctx.next_spec = -1;
 				}
@@ -3224,7 +3224,7 @@ void affect_spec(const runtime_state& ctx) {
 		case eSpecType::GIVE_ITEM:
 			if(pc_num >= 100) break;
 			if(spec.ex1a >= 0 && spec.ex1a < univ.scenario.scen_items.size()) {
-				cItem to_give = univ.scenario.scen_items[spec.ex1a];
+				cItem to_give = univ.get_item(spec.ex1a);
 				if(spec.ex1b >= 0 && spec.ex1b <= 6) {
 					// TODO: This array and accompanying calculation is now duplicated here, in start_town_mode(), and in place_buy_button()
 					const short aug_cost[10] = {4,7,10,8, 15,15,10, 0,0,0};
@@ -4030,7 +4030,7 @@ void townmode_spec(const runtime_state& ctx) {
 			position_party(spec.ex1a,spec.ex1b,spec.ex2a,spec.ex2b);
 			break;
 		case eSpecType::TOWN_PLACE_ITEM:
-			store_i = univ.scenario.get_stored_item(spec.ex2a);
+			store_i = univ.get_item(spec.ex2a);
 			place_item(store_i,l,spec.ex2b);
 			break;
 		case eSpecType::TOWN_SPLIT_PARTY:

src/game/boe.town.cpp

@@ -284,7 +284,7 @@ void start_town_mode(short which_town, short entry_dir) {
 		if(univ.town->preset_items[i].code >= 0) {
 			const cTown::cItem& preset = univ.town->preset_items[i];
 			// place the preset item, if party hasn't gotten it already
-			univ.town.items.push_back(univ.scenario.get_stored_item(preset.code));
+			univ.town.items.push_back(univ.get_item(preset.code));
 			cItem& item = univ.town.items.back();
 			item.item_loc = preset.loc;
 			

src/pcedit/pc.main.cpp

@@ -396,9 +396,9 @@ void handle_menu_choice(eMenu item_hit) {
 			break;
 		case eMenu::EDIT_ITEM:
 			if(scen_items_loaded) {
-				auto& all_items = univ.scenario.scen_items;
+				auto const & all_items = univ.scenario.scen_items;
 				std::vector<std::string> strings;
-				for(cItem& item : all_items) {
+				for(cItem const & item : all_items) {
 					strings.push_back(item.full_name);
 				}
 				cStringChoice dlog(strings, "Add which item?");

src/porting.cpp

@@ -227,8 +227,8 @@ void port_item_list(legacy::scen_item_data_type* old){
 	if(cur_file_is_mac != is_computer_small_endian)
 		return;
 	
-	for(short i = 0; i < 400; i++)
+	for(auto &item : old->scen_items)
-		port_item_record(&(old->scen_items[i]));
+		port_item_record(&item);
 }
 
 void port_out(legacy::outdoor_record_type *out) {

src/scenario/scenario.cpp

@@ -476,12 +476,6 @@ bool cScenario::is_item_used(item_num_t item) {
 	return false;
 }
 
-cItem cScenario::get_stored_item(int loot) {
-	if(loot >= 0 && loot < scen_items.size())
-		return scen_items[loot];
-	return cItem();
-}
-
 static const short loot_min[5] = {0,0,5,50,400};
 static const short loot_max[5] = {3,8,40,800,4000};
 
@@ -493,7 +487,7 @@ cItem cScenario::pull_item_of_type(unsigned int loot_max,short min_val,short max
 	}
 	for(short i = 0; i < 80; i++) {
 		int j = get_ran(1,0,scen_items.size() - 1);
-		cItem temp_i = get_stored_item(j);
+		cItem const &temp_i = get_item(j);
 		if(temp_i.variety == eItemType::NO_ITEM) continue;
 		if(std::find(types.begin(), types.end(), temp_i.variety) != types.end()) {
 			short val = (temp_i.charges > 0) ? temp_i.charges * temp_i.value : temp_i.value;

src/scenario/scenario.hpp

@@ -114,7 +114,6 @@ public:
 	bool is_ter_used(ter_num_t ter);
 	bool is_monst_used(mon_num_t monst);
 	bool is_item_used(item_num_t item);
-	cItem get_stored_item(int loot);
 	cItem return_treasure(int loot, bool allow_junk_treasure = false);
 	cItem pull_item_of_type(unsigned int loot_max,short min_val,short max_val,const std::vector<eItemType>& types,bool allow_junk_treasure=false);
 	

src/scenedit/scen.fileio.cpp

@@ -431,7 +431,7 @@ void writeItemsToXml(ticpp::Printer&& data, cScenario& scenario) {
 	for(size_t i = 0; i < scenario.scen_items.size(); i++) {
 		data.OpenElement("item");
 		data.PushAttribute("id", i);
-		cItem& item = scenario.scen_items[i];
+		cItem const &item = scenario.scen_items[i];
 		data.PushElement("variety", item.variety);
 		data.PushElement("level", item.item_level);
 		data.PushElement("awkward", item.awkward);

src/universe/universe.cpp

@@ -1446,7 +1446,7 @@ void cUniverse::refresh_store_items() {
 				if(choice < choices.size()) {
 					auto iter = choices.begin();
 					std::advance(iter, choice);
-					party.magic_store_items[i][j] = scenario.scen_items[*iter];
+					party.magic_store_items[i][j] = scenario.get_item(*iter);
 					continue;
 				}
 			} else if(entry.type == eShopItemType::OPT_ITEM) {

src/universe/universe.hpp

@@ -215,6 +215,8 @@ public:
 	explicit cUniverse(ePartyPreset party_type = PARTY_DEFAULT);
 	~cUniverse();
 
+	cItem const &get_item(item_num_t item) const { return scenario.get_item(item); }
+	cItem &get_item(item_num_t item) { return scenario.get_item(item); }
 	cTerrain const &get_terrain(ter_num_t ter) const { return scenario.get_terrain(ter); }
 	cTerrain &get_terrain(ter_num_t ter) { return scenario.get_terrain(ter); }