Problem z usuwaniem i czyszczeniem pamięci po wyjściu z "mapy"

0

Witam

Tworzę grę platformową i chciałbym po wyjściu z mapy do menu żeby cała mapa została zwalniana.

W zasadzie to wygląda to tak, że mam klasę GameScreen, która jest matką dla klas : menu główne, opcje, pauza, gra.

Ta klasa posiada parę metod w tym 2 :

void onEntry(); // < alokujemy co potrzeba
void onExit(); // < zwalniamy to co zaalokowaliśmy 

I do tego jest manager, który dodaje nam właśnie te klasy i ustawia, która klasa jest aktualnie aktywna.

Główna klasa z pętlą gry posiada metodę update, która w zależności od stanu, wywołuję onEntry() i onExit()

void MainGame::Update(float deltaTime)
	{
		if (currentScreen) {
			switch (currentScreen->getState()) {
				case ScreenState::RUNNING:
					currentScreen->Update(deltaTime); 
					break;
				case ScreenState::CHANGE_NEXT:
					currentScreen->onExit();
					currentScreen = screenManager->MoveNext();
					if (currentScreen) {
						currentScreen->setRunning();
						currentScreen->onEntry();
					}
					break;
				case ScreenState::CHANGE_PREVIOUS:
					currentScreen->onExit();
					currentScreen = screenManager->MovePrevious();
					if (currentScreen) {
						currentScreen->setRunning();
						currentScreen->onEntry();
					}
					break;
				case ScreenState::EXIT: 
					exitGame(); 
					break;
				default:
					break;
			}
		} else exitGame();
	}

W momencie kliknięcia escape kiedy jesteśmy na mapie, powinno przenosić nas do menu głównego a cała pamięć, która jest wykorzystywana do tworzenia tej mapy powinna zostać zwolniona.

Mam klasę Mapa która jest matką dla różnych typów oraz manager do tego:


class MapManager 
{
	... 
private:
	std::vector<std::unique_ptr<Map>> map;	
};

MapManager::MapManager() {}
MapManager::~MapManager() { Free(); }

void MapManager::AddMap(Map* m)
{
	map.push_back(std::unique_ptr<Map>(m));
}

void MapManager::Update(float deltaTime, Player& player)
{
	for (const auto& m : map) {
		m->Update(deltaTime, player);
	}

	/*for (auto &m : map)
	{
		auto removeIf = std::remove_if(map.begin(), map.end(), [](const std::unique_ptr<Map> &ma) {
			//return ma->mapState == Map::Destroy; TODO: create return
		});
		map.erase(removeIf, map.end());
	}*/
}

void MapManager::UpdateInput(InputManager input)
{
	for (const auto& m : map) {
		m->UpdateInput(input);
	}
}

void MapManager::Draw(Renderer& renderer)
{
	for (const auto& m : map) {
		m->Draw(renderer);
	}
}

int MapManager::getNumEnviroment()
{
	return (int)map.size();
}

void MapManager::Free()
{
	map.resize(0);
	for (auto& m : map) {
		m.reset();
	}
	map.clear();
}

I GameplayScreen, który onEntry dodaje mapę metodą void AddMap(Map* m);, a onExit powinien wywoływać metodę void Free();

Jednak gdy klikam escape i zaczyna dziać się cała magia pojawia się kod:

screenshot-20180219232913.png

nawet przy samym map.clear(); czy map.resize(0); pojawia się to samo.

Co jest tego problemem? Dlaczego nie moge zwolnić pamięci?

2

Jak wywołujesz metodę AddMap? Przyjmuje ona nagi wskaźnik i zamienia go na unique_ptr, wygląda to bardzo podejrzanie.

0

Jak wywołujesz metodę AddMap? Przyjmuje ona nagi wskaźnik i zamienia go na unique_ptr, wygląda to bardzo podejrzanie.

Tak, AddMap(new Light(someValues));

Co w tym podejrzanego?

Próbowałem zrobić to też metodą

void AddMap(int type) 
{
	switch(mapType) {
		case TYPE_LIGHT:
			map.push_back(std::unique_ptr<Map>(new Light(some values)));
			break;
	}
}

I wtedy AddMap(mapManager.LIGHT);

1

To co pokazałeś wygląda poprawnie, chociaż dzięki RAII nie potrzebujesz w ogóle destruktora MapManager, a jeśli już chcesz, to wystarczy map.resize(0).

Może dobierasz się do tego kodu z wielu wątków? Pokaż callstack w momencie błędu oraz funkcje onExit i onEntry

0

Nie wiem jak korzystać z wielowątkowości, nie uczyłem się jeszcze o tym więc raczej Może dobierasz się do tego kodu z wielu wątków? odpada.

screenshot-20180220005744.png

(Enviroment to jest klasa mapy. Zmieniłem sobie nazewnictwo w tym czasie już na przyszłość).

Przechodząc w Texture::~Texture(); jak Visual podpowiada, dostaję pusty destruktor.

onExit() wygląda tak jak napisałem

class GameplayScreen : public GameScreen
{
	...
	void onExit() override;
};

void GameplayScreen::onExit() 
{
	mapManager.Free();
}

void MapManager::Free()
{
	map.resize(0);
   	for (auto& m : map) {
        m.reset();
   	 }
   	 map.clear();
}

Właśnie uświadomiłem sobie, że nie potrzebuje wektora map skoro i tak gracz będzie się znajdować na jednej mapie. Więc przeprogramowałem map manager, tak, żeby był sam pointer i w metodzie AddMap(int mapType); alokował odpowiednią mapę w zależności od tego jaką wybierzemy.

Problem nadal zostaje, tym razem nawet jak usunę metodę void Free(); z void GameplayScreen::onExit() override; pojawia się to samo powiadomienie o błędzie.

0

1. To co pokazałeś jako stos wywołań zawiera tylko kod zewnętrzny. Byłoby super, gdybyś złapał ostatni moment w Twoim kodzie. Linijka Twojego kodu, po której wszystko się sypie.
2. Zastanawia mnie ten fragment:

void MapManager::AddMap(Map* m)
{
    map.push_back(std::unique_ptr<Map>(m));
}

Poprawcie mnie, jeśli się mylę, ale to co tutaj robisz to przyjmujesz adres na jakiś obiekt i przechowujesz ten adres.
Pytanie więc: Co się stanie, gdy w pewnym momencie obiekt ten umrze? Jego adres będziesz nadal przechowywał, ale adres ten niekoniecznie będzie już poprawny...

0

Co się stanie, gdy w pewnym momencie obiekt ten umrze?

W tym momencie zmienia stan na DESTROY (czy tam jaką inną nazwę chcemy) i następuje (racja, tego nie napisałem w poście, bo w tym czasie trochę dopisałem, odpisałem, ale o tym trochę niżej).

for (auto &m : map)
    {
        auto removeIf = std::remove_if(map.begin(), map.end(), [](const std::unique_ptr<Map> &ma) {
            return ma->mapState == Map::Destroy;
        });
        map.erase(removeIf, map.end());
    }

w void Update(...);

Ale jak już napisałem wyżej, nie ma sensu robić mapy jako tablica, bo gracz znajduje się na jednej mapie, a nie na kilku na raz, więc w zupełności Map* map wystarczy. W tym momencie metoda add map zmienia się i wygląda tak:

void MapManager::AddMap(int type) 
{
	switch(type) {
		case JAKASTAMMAPA:
			map = std::make_unique<JakaśTamMapa>(...);
			break;
		case JAKAS_TAM_DRUGA_MAPA:
			map = std::make_unique<JakaśTamDrugaMapa(...);
			break;
	}
}

W tym momencie nie mogę zrobić std::vector::resize(0); bo nie ma vektora, a ten sam błąd pokazuje się w momencie kiedy jestem na mapie i chce wrócić do menu głównego.

0

Pokaż cały kod

1

Drobna uwaga na szybko:

void MapManager::AddMap(int type) 

typ int lepiej byłoby zamienić na wyliczeniowy, w którym zawarłbyś wszystkie JAKASTAMMAPA.

Ponowię jednak pytanie: Czy potrafisz wskazać w swoim kodzie linijkę, po której wszystko się sypie? To by nam (i przede wszystkim Tobie) bardzo pomogło.

0

typ int lepiej byłoby zamienić na wyliczeniowy, w którym zawarłbyś wszystkie JAKASTAMMAPA.

Jest enum zrobiony.

Ponowię jednak pytanie: Czy potrafisz wskazać w swoim kodzie linijkę, po której wszystko się sypie? To by nam (i przede wszystkim Tobie) bardzo pomogło.

Tak. W momencie kiedy onExit() w klasie GameplayScreen jest wywoływane.

Pokaż cały kod

class MapManager
{
public:
	enum MapType {
		EARTH,
		FIRE,
		WATER,
		WIND
	};

	MapManager();
	~MapManager();

	void AddMap(MapType type);

	void Update(float deltaTime, Player& player);
	void Draw(Renderer& renderer);
private:
	std::unique_ptr<Map> map;
};
#include "MapManager.h"

MapManager::MapManager() {}
MapManager::~MapManager() {}

void MapManager::AddMap(MapType type)
{
	switch (type) {
		case EARTH:
			map = std::make_unique<EarthMap>("Files/Config/Maps/EarthMap.cfg", 10, 1);
			break;
		case FIRE:
			//map = std::make_unique<FireMap>("Files/Config/Maps/FireMap.cfg", 10, 1); < nie ma jeszcze tych plików, dlatego w komentarzu
			break;
		case WATER:
			//map = std::make_unique<WaterMap>("Files/Config/Maps/WaterMap.cfg", 10, 1);
			break;
		case WIND:
			//map = std::make_unique<WindMap>("Files/Config/Maps/WindMap.cfg", 10, 1);
			break;
	}
}

void MapManager::Update(float deltaTime, Player& player)
{
	map->Update(deltaTime, player);
}

void MapManager::Draw(MiEngine::Renderer& renderer)
{
	map->Draw(renderer);
}
class GameplayScreen : public GameScreen
{
public:
	GameplayScreen();
	~GameplayScreen();

	int getNextScreenIndex() const override;
	int getPreviousScreenIndex() const override;
	
	void onEntry() override;
	void onExit() override;

	void UpdateInput(InputManager input) override;
	void Update(float deltaTime) override;

	void Draw(Renderer& renderer) override;

private:
	Player player;
	MapManager mapManager;
};
GameplayScreen::GameplayScreen() {}
GameplayScreen::~GameplayScreen() {}

int GameplayScreen::getNextScreenIndex() const
{
	return SCREEN_NONE;
}

int GameplayScreen::getPreviousScreenIndex() const
{
	return MainOptions::PAUSEMENU;
}

void GameplayScreen::onEntry()
{
	mapManager.AddMap(mapManager.EARTH);
	player.Initialize(500, 500);
}

void GameplayScreen::onExit() // !!!!!! W TYM MOMENCIE 
{
	player.Free(); // < I player.Free(); nie ma z tym nic wspólnego, bo bez tego też nie działa, a jak usunę MapManager i zostawie samego gracza wszystko ładnie śmiga.
}

void GameplayScreen::Update(float deltaTime)
{
	mapManager.Update(deltaTime, player);
	player.Update(deltaTime);
}

void GameplayScreen::UpdateInput(MiEngine::InputManager input)
{
	if (input.KeyDown(KEY_ESCAPE)) {
		currentState = ScreenState::CHANGE_PREVIOUS; // !!!!!!! A TEN MOMENT JEST DZIĘKI KLIKNIĘCIU TEGO KLAWISZA
	}
}

void GameplayScreen::Draw(MiEngine::Renderer& renderer)
{
	mapManager.Draw(renderer);
	player.Draw(renderer);
}

Game loop

class MainGame
	{
	public:
		MainGame();
		virtual ~MainGame();

		void exitGame();

		virtual void mainLoop();

		virtual void addScreens() = 0;
	protected:
		virtual void Update(float deltaTime);

		virtual void Draw();
		virtual void Draw(Renderer& renderer);

		virtual bool init();
		virtual void InitShader();

		Window window;
		Shader shader;
		Renderer renderer;
		Renderer textRenderer;
		Camera2D camera2D;
		Camera2D textCamera2D;
		TimeStep time;
		InputManager input;

		std::unique_ptr<ScreenManager> screenManager;
		GameScreen* currentScreen = nullptr;
		bool quit = false;	
	};
	MainGame::MainGame() 
	{
		//window.CreateWindow("default", 1280, 720);
		screenManager = std::make_unique<ScreenManager>();
	}

	MainGame::~MainGame() { exitGame(); }

	void MainGame::exitGame() 
	{
		currentScreen->onExit();
		if (screenManager) {
			screenManager->Free();
			screenManager.reset();
		}
		quit = true;
		delete currentScreen;
		window.DestroyWindow();
		SDL_Quit();
		exit(0);
	}

	void MainGame::mainLoop()
	{
		glClearColor(0.5f, 0.5f, 0.5f, 1.0f);

		time.Init();

		while (!quit) {
			time.Begin();
			while (window.PollEvent()) {
				if (window.event.type == SDL_QUIT) {
					SDL_Quit();
					exit(0);
				}

				input.UpdateEvent(window.event);
				currentScreen->UpdateInput(input);
			}
			GLint u_camera = shader.GetUniformLocation("u_camera");
			glm::mat4 cameraMatrix = camera2D.getCameraMatrix();

			glUniformMatrix4fv(u_camera, 1, GL_FALSE, &(cameraMatrix[0][0]));
	
			while (time.OnUpdate()) {
				Update(time.getDeltaTime());
				time.End();
			}

			renderer.RenderClear();
			textRenderer.RenderClear();

			Draw();

			renderer.Render();
			textRenderer.Render();

			window.SwapWindow();
		}
	}

	bool MainGame::init() 
	{		
		addScreens();
		if (!init()) return false;

		currentScreen = screenManager->getCurrent();
		currentScreen->onEntry();
		currentScreen->setRunning();

		renderer.CreateVertexArray();
		textRenderer.CreateVertexArray();

		InitShader();
	
		return true;
	}

	void MainGame::InitShader()
	{
		std::string vertex = shader.LoadFromFile("vertex.shader");
		std::string fragment = shader.LoadFromFile("fragment.shader");

		shader.CreateShaderProgram();
		shader.CreateShaders(vertex.c_str(), fragment.c_str());
		shader.UseShaderProgram();

		GLint u_textureSampler = shader.GetUniformLocation("u_textureSampler");
		glUniform1f(u_textureSampler, 0);
	}

	void MainGame::Update(float deltaTime)
	{
		if (currentScreen) {
			switch (currentScreen->getState()) {
				case ScreenState::RUNNING:
					currentScreen->Update(deltaTime); 
					break;
				case ScreenState::CHANGE_NEXT:
					currentScreen->onExit();
					currentScreen = screenManager->MoveNext();
					if (currentScreen) {
						currentScreen->setRunning();
						currentScreen->onEntry();
					}
					break;
				case ScreenState::CHANGE_PREVIOUS:
					currentScreen->onExit();
					currentScreen = screenManager->MovePrevious();
					if (currentScreen) {
						currentScreen->setRunning();
						currentScreen->onEntry();
					}
					break;
				case ScreenState::EXIT: 
					exitGame(); 
					break;
				default:
					break;
			}
		} else exitGame();
	}

	void MainGame::Draw()
	{
		glViewport(0, 0, window.getScreenWidth(), window.getScreenHeight());
		if (currentScreen && currentScreen->getState() == ScreenState::RUNNING) {
			currentScreen->Draw(renderer);
			currentScreen->DrawText(textRenderer);
		}
	}
0

Poczekaj...

< I player.Free(); nie ma z tym nic wspólnego, bo bez tego też nie działa, a jak usunę MapManager i zostawie samego gracza wszystko ładnie śmiga.

Upewniam się: wersja, którą wysłałeś nie działa, ale jest jeszcze wersja bez player.Free() która też nie działa. Czyli:

void GameplayScreen::onExit() // !!!!!! W TYM MOMENCIE 
{
}

^ Takie coś też nie działa? Staram się dobrze Ciebie zrozumieć.

Czy mógłbyś też pokazać zmienne lokalne po wskoczeniu w tą metodę? Lub widok na jakiekolwiek zmienne, które mogą być wykorzystywane w chwili wysypania się?

0

screenshot-20180220135000.png

0

ja obstawiam

delete currentScreen;

ale to tylko z powierzchownych oględziń kodu

edit:
jest tam zwalniany ScreenManager, który już mógł się tym zająć

0

Tylko jak mam różne menu to nie ma problemu z wychodzeniem z niego.

class MenuList
	{
	public:
		MenuList(float x, float y, float fontSize, std::string text)
		{
			this->x = x;
			this->y = y;
			this->fontSize = fontSize;
			this->text = text;
		}

		MenuList(float x, float y, float fontSize, std::string text, ColorRGBA color)
		{
			this->x = x;
			this->y = y;
			this->fontSize = fontSize;
			this->text = text;
			this->color = color;
		}
	
		float x = 0.0f;
		float y = 0.0f;
		float fontSize = 0;
		std::string text = "";
		ColorRGBA color = {255, 255, 255, 255};
	};

	class Menu : public MiEngine::GameScreen
	{
	public:
		Menu();
		virtual ~Menu();

		virtual void AddOption(float x, float y, float fontSize, std::string text);
		virtual void AddOption(float x, float y, float fontSize, std::string text, ColorRGBA color);

		virtual void DrawSingle(Renderer& renderer, int index);
		virtual void DrawSingle(Renderer& renderer, int index, ColorRGBA color);

		virtual void UpdateInput(MiEngine::InputManager input) override;

		virtual void DrawText(Renderer& renderer) override;
		virtual void DrawText(Renderer& renderer, ColorRGBA color) override;

		virtual void onEntry() override;
		virtual void onExit() override;
	protected:

		virtual void UpdateKeyboard(InputManager input);
		virtual void UpdateMouse(InputManager input);

		std::unique_ptr<Text> text = nullptr;
		std::vector<std::unique_ptr<MenuList>> menuList;

		int nextScreenIndex = 0;
		int prevScreenIndex = 0;

		int activeOption = 0;

		bool keyPressed = false;
		bool mousePressed = false;

		float x, y;
		const float dth = 75.0f;
	};
	void Menu::onExit() 
	{
		for (auto& ml : menuList) {
			ml.reset();
		}
		menuList.clear();
		menuList.resize(0); // <Tutaj nie jestem pewien czy clear nie powinno być po resize,
	}
class MainMenu : public Menu
{
public:
	enum {
		NEWGAME = 0,
		LOADGAME = 1,
		OPTIONS = 2,
		CREDITS = 3,
		EXIT = 4
	} MenuWays;

	MainMenu();
	~MainMenu();

	int getNextScreenIndex() const override;
	int getPreviousScreenIndex() const override;

	void onEntry() override;
	void onExit() override;

	void UpdateInput(InputManager input) override;
	void DrawText(Renderer& renderer) override;

private:
	int leftBound = 100;
	int rightBound = 440;

	bool OnNewGameClicked();
	bool OnLoadGameClicked();
	bool OnOptionsClicked();
	bool OnCreditsClicked();
	bool OnExitClicked();

	void UpdateKeyboard(InputManager input) override;
	void UpdateMouse(InputManager input) override;
};
MainMenu::MainMenu() {}
MainMenu::~MainMenu() {}

int MainMenu::getNextScreenIndex() const
{
	return nextScreenIndex;
}

int MainMenu::getPreviousScreenIndex() const
{
	return SCREEN_NONE;
}

void MainMenu::onEntry()
{
	Menu::onEntry();
	lastScreenIndex = MainOptions::MAINMENU;

	x = (float)leftBound;
	y = 525;
	              AddOption(x, y, 10, "New Game");	                            // < 0
	y -= dth; AddOption(x, y, 10, "Load Game");	    // < 1
	y -= dth; AddOption(x, y, 10, "Options");	            // < 2
	y -= dth; AddOption(x, y, 10, "Credits");	            // < 3
	y -= dth; AddOption(x, y, 10, "Exit Game");	    // < 4
}

void MainMenu::onExit()
{
	Menu::onExit();
}

Reszta metod małe ma znaczenie.

Klasy takie jak Opcje czy creditsy działają na podobnej zasadzie i w momencie przejścia z MainMenu do opcji zostaje odpalona onExit z metody MainMenu bez żadnych problemów. Kiedy przechodzę z opcji do MainMenu - onExit() odpala się z klasy opcji

void OptionsMenu::onExit()
{
	Menu::onExit();
}

I też bez problemu przechodzi do MainMenu. Sprawdzałem parę razy i nigdy nie wywaliło mi błędu.

1 użytkowników online, w tym zalogowanych: 0, gości: 1