From 518ea93adc9dd5afa2b9cb3c4f5c49286ff4f4bc Mon Sep 17 00:00:00 2001
From: Kayne Ruse <kayneruse@gmail.com>
Date: Thu, 25 Aug 2016 20:47:08 +1000
Subject: [PATCH] Added RoomData::[Pop/Push]Character()

---
 server/rooms/room_data.cpp    | 31 +++++++++++++++++++++++++++++++
 server/rooms/room_data.hpp    |  5 ++++-
 server/rooms/room_manager.cpp | 16 +++++++---------
 server/rooms/room_manager.hpp |  4 ++--
 server/server_utilities.cpp   |  2 +-
 5 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/server/rooms/room_data.cpp b/server/rooms/room_data.cpp
index 0c04514..559d637 100644
--- a/server/rooms/room_data.cpp
+++ b/server/rooms/room_data.cpp
@@ -257,6 +257,37 @@ int RoomData::GetRoomIndex() {
 	return roomIndex;
 }
 
+void RoomData::PushCharacter(CharacterData* const character) {
+	if (!character) {
+		throw(std::logic_error("Failed to push a null character to RoomData"));
+	}
+
+	//check to see if the character is already here
+	for (auto& it : characterList) {
+		if (it == character) {
+			throw(std::logic_error("double insertion of CharacterData in RoomData"));
+		}
+	}
+
+	characterList.push_back(character);
+}
+
+void RoomData::PopCharacter(CharacterData const * const character) {
+	if (!character) {
+		throw(std::logic_error("Failed to pop a null character to RoomData"));
+	}
+
+	//check to see if the character isn't here
+	for (auto& it : characterList) {
+		if (it == character) {
+			characterList.remove(it);
+			return;
+		}
+	}
+
+	throw(std::logic_error("cannot remove a non-existant instance of CharacterData in RoomData"));
+}
+
 BarrierManager* RoomData::GetBarrierMgr() {
 	return &barrierMgr;
 }
diff --git a/server/rooms/room_data.hpp b/server/rooms/room_data.hpp
index ccac6ca..d6b0494 100644
--- a/server/rooms/room_data.hpp
+++ b/server/rooms/room_data.hpp
@@ -53,6 +53,9 @@ public:
 	int SetRoomIndex(int i);
 	int GetRoomIndex();
 
+	void PushCharacter(CharacterData* const character);
+	void PopCharacter(CharacterData const * const character);
+
 	BarrierManager* GetBarrierMgr();
 	std::list<CharacterData*>* GetCharacterList();
 	CreatureManager* GetCreatureMgr();
@@ -86,7 +89,7 @@ private:
 	std::string tilesetName;
 
 	//members
-	int roomIndex = 0;
+	int roomIndex = 0; //NOTE: What's this doing here?
 	BarrierManager barrierMgr;
 	std::list<CharacterData*> characterList;
 	BattleManager battleMgr;
diff --git a/server/rooms/room_manager.cpp b/server/rooms/room_manager.cpp
index d35af54..f6373b1 100644
--- a/server/rooms/room_manager.cpp
+++ b/server/rooms/room_manager.cpp
@@ -111,7 +111,7 @@ void RoomManager::UnloadIf(std::function<bool(std::pair<const int, RoomData cons
 	lua_pop(lua, 1);
 }
 
-void RoomManager::PushCharacter(CharacterData* character) {
+void RoomManager::PushCharacter(CharacterData* const character) {
 	if (!character) {
 		throw(std::runtime_error("Failed to push a null character to a room"));
 	}
@@ -119,27 +119,25 @@ void RoomManager::PushCharacter(CharacterData* character) {
 	RoomData* room = Find(character->GetRoomIndex());
 
 	if (!room) {
-		throw(std::runtime_error("Failed to push an character to a non-existant room"));
+		throw(std::runtime_error("Failed to push a character to a non-existant room"));
 	}
 
-	room->GetCharacterList()->push_back(character);
+	room->PushCharacter(character);
 }
 
-void RoomManager::PopCharacter(CharacterData const* character) {
+void RoomManager::PopCharacter(CharacterData const * const character) {
 	//NOTE: to pop an character from a room, the character must first exist
 	if (!character) {
-		throw(std::runtime_error("Failed to pop a null character to a room"));
+		throw(std::runtime_error("Failed to pop a null character from a room"));
 	}
 
 	RoomData* room = Find(character->GetRoomIndex());
 
 	if (!room) {
-		throw(std::runtime_error("Failed to pop an character to a non-existant room"));
+		throw(std::runtime_error("Failed to pop a character to a non-existant room"));
 	}
 
-	room->GetCharacterList()->remove_if([character](CharacterData* ptr) {
-		return character == ptr;
-	});
+	room->PopCharacter(character);
 }
 
 //TODO: rename these functions from Get to Find
diff --git a/server/rooms/room_manager.hpp b/server/rooms/room_manager.hpp
index 9243ed8..f6dea1c 100644
--- a/server/rooms/room_manager.hpp
+++ b/server/rooms/room_manager.hpp
@@ -39,8 +39,8 @@ public:
 	void UnloadAll();
 	void UnloadIf(std::function<bool(std::pair<const int, RoomData const&>)> fn);
 
-	void PushCharacter(CharacterData* character);
-	void PopCharacter(CharacterData const* character);
+	void PushCharacter(CharacterData* const character);
+	void PopCharacter(CharacterData const * const character);
 
 	//accessors and mutators
 	RoomData* Find(int uid);
diff --git a/server/server_utilities.cpp b/server/server_utilities.cpp
index a1d353d..c866571 100644
--- a/server/server_utilities.cpp
+++ b/server/server_utilities.cpp
@@ -195,7 +195,7 @@ void pumpAndChangeRooms(int characterIndex, int newRoomIndex) {
 	pumpAndChangeRooms(character, newRoomIndex, characterIndex);
 }
 
-//TODO: (0) refactor this
+//NOTE: This is one of those ugly things that you just need to put up with
 void pumpAndChangeRooms(CharacterData* const characterData, int newRoomIndex, int characterIndex) {
 	//delete the character from the old room
 	CharacterPacket newPacket;