Listen extern iterieren

Guten Tag Leute,

Ich habe folgendes Problem, ich möchte von einer Klasse GravityHandler (public) auf eine die Liste der Klasse Galaxy (public) zugreifen und durch die Liste (Planetenliste) iterieren, allerdings kann ich nicht drauf zugreifen, da es Speicherverletzungen zu geben scheint, das Programm beendet dann einfach.

Hier der Code den ich dafür benutze:

for (std::list<Planet*>::iterator it = galaxy->planetList.begin(); it !=    galaxy->planetList.end(); ++it)
{

}

Ich hatte die PlanetenList einfach als public deklariert, nachdem es auch mit einem Getter nicht ging, das Resultat ist das gleiche.

Ich kann diese Iteration ohne weiteres auf das Objekt Galaxy mit einer Funktion auslagern, allerdings möchte ich es designtechnisch so lösen.

Ich weiß das Problem ist etwas speziell und vielleicht konnte ich es auch nicht richtig darbieten, vielen Dank für eure Hilfe.

Ein Container mit Raw-Pointern

std::list<Planet*>

ist immer eine Schwachstelle. Empfehlung std::shared_ptr<Planet>

Verdacht: ein (oder mehrere) Pointer in der Liste sind ungültig.

Eine std::list von Pointern ist Overhead pur. Nimm vector<Planet*> oder besser vector<shared_ptr<Planet>>.

Eisflamme

Also ein Container mit Raw-Pointern ist total okay, wenn es sich einfach nur um Verweise handelt (dann natürlich vector >> list). Aber die Implikation, dass es doch besitzende Zeiger sind, liegt nahe.

Wieso hier aber alle ohne irgendwelche Kontextinformationen den overheadbehafteten shared_ptr empfehlen, verstehe ich nicht.

list->vector:
Wie kannst Du einerseits list wegen Overhead verurteilen, aber shared_ptr empfehlen?!

@TE:
An dem Code ist nichts falsch (nur etwas umständlich), wir brauchen schon weitere Informationen. Debug Mal rein, um zu sehen, wo er wirklich zusammenbricht.

Die Glaskugel zeigt: Im Destruktor von Galaxy werden alle Planeten gelöscht. Ein Objekt der Klasse Galaxy wurde kopiert (evt. auch unbewusst)

FroZenViper

Also Shared_ptr sind im Allgemeinen ja schon einfacher da es keine Speicherlecks gibt etc., oder schwerer sind diese herbeizurufen.

Habe grad mal probiert einfach im Galaxy Objekt einen Iteratortest anzulegen, der funktioniert auch auf der Klasse.

Ich kann mir kaum vorstellen dass es was mit den planet-pointern zu tun hat, da ich z.B. wenn ich alles auskommentiere in der for schleife schon den Fehler bekomme, d.h. es wird zu keinem Zeitpunkt auf die Zeiger im Container zugegriffen.
Außer mit dem Begin und End, keine Ahnung wie sich das verhält.

Ich habe genau ein Element in der Liste, welches ich mit Push_back in der Main einfüge und das Objekt besteht auf jeden Fall, ist also nicht null.

Debug Informationen sind, wobei ich nicht weiß ob die euch was nutzen.

"Unbehandelte Ausnahme bei 0x0129DA06 in Party Planet (work in progress).exe: 0xC0000005: Zugriffsverletzung beim Lesen an Position 0xCDCDCDCD"

http://i.imagebanana.com/img/u9wjfcgb/thumb/Fehler.PNG

ich glaub halt einfach dass das irgendwas mit der Liste an sich zu tun hat, bei dem man einen Iterator einer Liste nicht auf einem anderen Objekt, als das in der die Liste drin ist, erstellen kann, oder so etwas in der Art, aber ist nur eine Vermutung.

Eisflamme

Zeig Mal mehr Code, insbesondere den Teil, bei dem es dumpt. Dein Codeausschnitt ist unzureichend.

Die Debug-Fehlermeldung sagt einfach, dass der Zeiger uninitialisiert ist (nicht mehr oder noch nicht) und dass Du trotzdem versuchst über diesen Zeiger auf ein Objekt zuzugreifen. Zeig auch Mal deinen Push-Code von Der Main, nutzt Du überhaupt new?

Statt shared_ptr würde ich einfach unique_ptr empfehlen und fertig, der ist sehr vernünftig.

FroZenViper schrieb:

ich glaub halt einfach dass das irgendwas mit der Liste an sich zu tun hat, bei dem man einen Iterator einer Liste nicht auf einem anderen Objekt, als das in der die Liste drin ist, erstellen kann, oder so etwas in der Art, aber ist nur eine Vermutung.

Nein, das kann nicht sein.

Die naheliegende Vermutung ist, dass du irgendwo deinen Heap kaputt gemacht machst, weil du die Dreierregel (Wikipedia: Rule of Three) verletzt hast. Zeig uns mehr Code (zum Beispiel da, wo du deine Liste erstellst) und wir werden dir den Fehler zeigen können.

FroZenViper schrieb:

"Unbehandelte Ausnahme bei 0x0129DA06 in Party Planet (work in progress).exe: 0xC0000005: Zugriffsverletzung beim Lesen an Position 0xCDCDCDCD"

dabei handelt es sich um uninitialisierten Speicher auf dem Heap

FroZenViper schrieb:

http://i.imagebanana.com/img/u9wjfcgb/thumb/Fehler.PNG

Der Fehler tritt bei einer Dereferenzierung des list-Iterators auf. Das widerspricht sich mit Deiner Aussage:

FroZenViper schrieb:

.. wenn ich alles auskommentiere in der for schleife schon den Fehler bekomme,..

Existiert das Galaxy-Objekt zu diesem Zeitpunkt noch?

FroZenViper

okay eig. wollte ich es vermeiden so viel Code zu senden,

das ist die Main, das ganze läuft mit sfml und sf ist namespace

using namespace sf;

int main()
{
	float alpha = 0.0f;

	VideoMode desktop = VideoMode::getDesktopMode();
	RenderWindow window(desktop,GAMENAME, Style::Fullscreen);

	//Creation of Player and one Planet
	Vector2f planetPosition;
	planetPosition.x = (float) desktop.width/2;
	planetPosition.y = (float) desktop.height/2;
	Planet* planet = new Planet(planetPosition, 200.0f);
	Player* player = new Player();

	//Gravitation
	RadialGravity* gravity = new RadialGravity(400, 1, 5); 

	//A new Galaxy
	Galaxy* galaxy = new Galaxy();
	galaxy->getPlanetList()->push_back(planet);

	//Set Position of Player
	Vector2f playerPosition;
	playerPosition.x = planet->getPlanet()->getPosition().x + planet->getPlanet()->getRadius();
	playerPosition.y =  planet->getPlanet()->getPosition().y - 40.0f;
	player->setPlayerPosition(playerPosition);
	player->setNewGalaxy(galaxy);
	//player->getPlayer()->move(planet->getPlanet()->getRadius()-15.0f,-40.0f);

	/* float yPositionPlayer = planet->getPlanet()->getRadius()+40.0f;
	float ySpeed = 1.0f;
	float graviNumber = 0.002f;
	float yGravitation;
	bool jump = false; */

	while (window.isOpen() && !Keyboard::isKeyPressed(Keyboard::Key::Escape))
    {
        Event event;
        while (window.pollEvent(event))
        {
            if (event.type == Event::Closed)
                window.close();
        }

		window.clear();
		planet->draw(window,RenderStates::Default);
		player->draw(window,RenderStates::Default);

		player->update();
             ...

das ist die Galaxy.h

class Galaxy 
{
	public:
		std::list<Planet*>*  getPlanetList();
		void iteratorTest();
	private:
		String galaxyName;
		std::list<Planet*> planetList;
};

der Getter in der Galaxy.cpp

std::list<Planet*>* Galaxy::getPlanetList()
{
	std::list<Planet*>* planetListPointer = &planetList;
	return planetListPointer;
}

im Player wird update aufgerufen das sieht man im Code ganz unten, und in dem Update spricht er den GravityHandler an und da gibts den Fehler, weil der auf die Galaxien Liste zugreift

void Player::update() 
{
	//whats the gravitation?
	//gravityHandler->galaxy->iteratorTest();
	this->gravityHandler->activeGravitation();
	//updateMovement();
}

GravityHandler Klasse

class GravityHandler 
{
	public:
		GravityHandler(Player* player);
		void setGalaxy(Galaxy* galaxy);
		Gravity* activeGravitation();
		Vector2f calculateGravityPlayerPosition();
		float calculateGravityPlayerRotation();
		Galaxy* galaxy;
	private:
		float calculateGravityPlayerHeight();
		float calculateGravityPower();
		Gravity* activeGravity;
		Planet* activeGravityObject;
		//Galaxy* galaxy;
		Player* player;
};

den Code betreffende CPP code

Gravity* GravityHandler::activeGravitation()
{
	//return value
	Gravity* theActiveGravitation = NULL;

	//position of the player
	Vector2f playerPosition = player->getPlayerPosition();

	//Are the radial planet gravity fields in range?
	for (std::list<Planet*>::iterator it = galaxy->getPlanetList()->begin(); it != galaxy->getPlanetList()->end(); ++it)
	{
			Vector2f differencePositions;

			differencePositions.x = (*it)->getPosition().x - playerPosition.x;
			differencePositions.y = (*it)->getPosition().y - playerPosition.y;
			float distance = sqrt( (differencePositions.x * differencePositions.x) + (differencePositions.y * differencePositions.y));

			//yes for this member it is in range
			if(distance <= (*it)->getGravity()->getRange() && (*it)->getGravity() != this->activeGravity)
			{
				//saves the gravity in handler, return value and saves the object this gravity belongs to
				activeGravity = (*it)->getGravity();
				theActiveGravitation = (*it)->getGravity();
				this->activeGravityObject = (*it); 
			}

	}

	//Wählt das neue Gravitationsfeld aus
	return theActiveGravitation;
}

ich hoffe das bringt was, sry für die wall of text

Der Member 'galaxy' wird zwar im Player gesetzt, aber nicht im GravityHandler!

FroZenViper

fuck, vielen Dank :S manche Fehler sind so trivial. Also danke an alle die mitgeholfen haben, werde das mal schleunigst korrigieren.

Und wenn du gerade schon dabei zu verbessern:
Wenn du alpha auch noch auf dem Heap erzeugst hast du wenigstens konsequent alles auf dem Heap liegen... naja, fast.
Kommst du aus der Java/C# Ecke?

using namespace sf; 

int main() 
{ 
    float alpha = new float( 0.0f ); 

    VideoMode desktop = VideoMode::getDesktopMode(); 
    RenderWindow window(desktop,GAMENAME, Style::Fullscreen); 

    //Creation of Player and one Planet 
    Vector2f planetPosition; 
    planetPosition.x = (float) desktop.width/2; 
    planetPosition.y = (float) desktop.height/2; 
    Planet* planet = new Planet(planetPosition, 200.0f); 
    Player* player = new Player(); 

    //Gravitation 
    RadialGravity* gravity = new RadialGravity(400, 1, 5); 

    //A new Galaxy 
    Galaxy* galaxy = new Galaxy(); 
    galaxy->getPlanetList()->push_back(planet);

    ...
}

FroZenViper

danke, für den Hinweis, werde ich auch noch machen. Naja Java/c#, sogar basic und delphi, hab ich auch schon geschrieben, eigentlich alles Querfeld, merkt man wahrscheinlich schon an der ein oder anderen Stelle.

DocShoe

Ich ~~denke~~ hoffe, das war ironisch gemeint. Du solltest eigentlich vermeiden, Daten explizit auf dem Heap zu erzeugen. Das gilt gerade für Galaxy , Planet und Player , die könnten genauso gut auf dem Stack liegen.

FroZenViper

das war leider nicht mal der Fehler, ich habs ein bisschen trickreich in meinem Code gemacht

void Player::setNewGalaxy(Galaxy* galaxy)
{
	this->gravityHandler->setGalaxy(galaxy);
}

man setzt das Level beim Spieler und der setzt es bei der Gravitation

FroZenViper

DocShoe schrieb:

Ich ~~denke~~ hoffe, das war ironisch gemeint. Du solltest eigentlich vermeiden, Daten explizit auf dem Heap zu erzeugen. Das gilt gerade für Galaxy , Planet und Player , die könnten genauso gut auf dem Stack liegen.

ja okay, ich muss mich da mal schlau machen, mit dem Unterschied der Erzeugung etc., bin halt kein Profi.

camper

FroZenViper schrieb:

danke, für den Hinweis, werde ich auch noch machen. Naja Java/c#, sogar basic und delphi, hab ich auch schon geschrieben, eigentlich alles Querfeld, merkt man wahrscheinlich schon an der ein oder anderen Stelle.

Zeiger+new und "Handler" sind ein Indiz. Ich würde GravityHandler ja zumindest durch GravityForce ersetzen, dann fällt nicht so auf, dass die Gravitation nicht selbst agieren kann.

FroZenViper schrieb:

Also Shared_ptr sind im Allgemeinen ja schon einfacher da es keine Speicherlecks gibt etc., oder schwerer sind diese herbeizurufen.

Ja, aber shared_ptr ist für geteilten Besitz. Den hast du nicht.

Für RAII bitte unique_ptr nehmen.

Unique Pointer Commitee schrieb:

Für RAII bitte unique_ptr nehmen.

unique_ptr ist aber für Polymorphie. Den hat er hier nicht.

Für RAII bitte vector<Planet> nehmen.