Circular inclusion problem

Recommended Posts

I think I am having the problem mentioned in the title, but I can't be sure of it as well...

Got 3 classes, GameMode, Entity, Paddle. 

Paddle Inherit from Entity, and both Paddle and Entity need to #include "GameMode.h" in order to pass down the constructor a GameMode* and store it inside of every Entity.

I pretty much forward declared everything inside everything else but stuff just won't compile, the error I am getting is C2504 "Entity: base class undefined" .

I'll paste below the .h and .cpp for the Entity class, and only .h for the other two. Can someone tell me what am I doing wrong? :/

Entity.h

#pragma once
#include "SDL2\SDL.h"
#include "GameMode.h"
#include "Utility.h"
using namespace util;

class GameMode;
enum class PivotMode: Uint8 {CENTER,TOP_LEFT};

class Entity
{
protected://variables
	float XCenter;
	float YCenter;
	SDL_Rect CollisionBox;
	SDL_Texture* Sprite;
	GameMode* Game;
	SDL_Renderer* Renderer;

public://constructors
	Entity(GameMode* gameRef, PivotMode inputMode, int x, int y, std::string path);
	virtual ~Entity();
	Entity(const Entity&) = delete;
	Entity& operator=(const Entity&) = delete;
	Entity(Entity&&) = delete;
	Entity& operator=(Entity&&) = delete;

public://methods
	virtual void Update(float deltaTime) = 0;
	virtual void Draw(float interpolation) = 0;

private://methods
SDL_Texture* RequestTexture(std::string path)const;
void SetCollisionBox(int x, int y, PivotMode InputMode);
};

Entity.cpp

#include "Entity.h"

Entity::Entity(GameMode* gameRef, PivotMode inputMode, int x, int y, std::string path)
	:Game{ gameRef }, XCenter{ static_cast<float>(x) }, YCenter{ static_cast<float>(y) }
{
	if (Game) { Renderer = Game->GetRenderer(); }
	Sprite = RequestTexture(path); 

	if (Sprite)
	{ SetCollisionBox(x, y, inputMode); }
}

Entity::~Entity()
{
}

SDL_Texture* Entity::RequestTexture(std::string path)const
{
	if (Game->IsRunning())
	{
		return Game->RequestTexture(path);
	}
	return nullptr;
}

void Entity::SetCollisionBox(int x, int y, PivotMode InputMode)
{
	SDL_QueryTexture(Sprite, nullptr, nullptr, &CollisionBox.w, &CollisionBox.h);

	switch (InputMode)
	{
		case PivotMode::CENTER:
		{
			CollisionBox.x = x - CollisionBox.w / 2;
			CollisionBox.y = y - CollisionBox.h / 2;
			XCenter = static_cast<float>(x);
			YCenter = static_cast<float>(y);
		}break;
		case PivotMode::TOP_LEFT:
		{
			CollisionBox.x = x;
			CollisionBox.y = y;
			XCenter = static_cast<float>(x) + CollisionBox.w / 2;
			YCenter = static_cast<float>(y) + CollisionBox.h / 2;
		}break;
	}
}

Paddle.h

#pragma once
#include "Entity.h"
#include "GameMode.h"

class GameMode;
enum class PivotMode:Uint8;

class Paddle : public Entity
{
public://methods
	Paddle(GameMode* gameRef, PivotMode inputMode, int x, int y, std::string path);
	~Paddle();

	virtual void Update(float deltaTime);
	virtual void Draw(float interpolation);
};

GameMode.h

#pragma once
#include "SDL2\SDL.h"
#include <string>
#include <vector>
#include <memory>
#include "App.h"
#include "Entity.h"
#include "Paddle.h"

class Entity;
class Paddle;

class GameMode
{
	friend class App;
private://variables
	bool Running;
	SDL_Window* Window;
	SDL_Renderer* Renderer;
	App* AppRef;

public://constructors
	GameMode(SDL_Window* Window, SDL_Renderer* Renderer, App* App);
	~GameMode();

	GameMode(const GameMode&) = delete;
	GameMode& operator=(const GameMode&) = delete;	
	GameMode(GameMode&&) = delete;
	GameMode& operator=(GameMode&&) = delete;

public://methods
	SDL_Texture* RequestTexture(std::string path)const;
	bool IsRunning()const;
	SDL_Renderer* GetRenderer()const;

private://methods
	void Update(float deltaTime);
	void Draw(float interpolation);
};

 

Share this post


Link to post
Share on other sites
4 minutes ago, MarcusAseth said:

Paddle Inherit from Entity, and both Paddle and Entity need to #include "GameMode.h" in order to pass down the constructor a GameMode* and store it inside of every Entity.

No they don't. You can forward declare GameMode if all you need is to refer to it by pointer.

 

Then just include GameMode.h in the CPP files.

Share this post


Link to post
Share on other sites
7 minutes ago, MarcusAseth said:

Paddle Inherit from Entity, and both Paddle and Entity need to #include "GameMode.h" in order to pass down the constructor a GameMode* and store it inside of every Entity.

 

No they don't; the forward declaration you have of GameMode is sufficient to declare pointers to GameMode. You only need to include the header if you need anything that requires the full definition of GameMode, which you don't unless you actually try to call a method from it, take its size, et cetera. Generally you can defer the inclusion of GameMode.h to the .cpp in this case.

What you are seeing here is that during the compilation of Entity.cpp, you include Entity.h (which, remember, is basically just pasting the content of Entity.h into Entity.cpp -- the compiler is only compiling the TU resulting from preprocessing Entity.cpp). Entity.h includes SDL stuff which is irrelevant, and then includes GameMode.h. GameMode.h. GameMode.h includes a bunch of crap, and then Entity.h (whcih is skipped due to the pragma) and then Paddle.h. Paddle.h doesn't do anything with the include of Entity.h due to the aforementioned pragma.

Thus the compiler is seeing, from top-to-bottom (with irrelevant stuff removed):

  • the content of paddle.h
  • the content of gamemode.h
  • the content of entity.h
  • the content of entity.cpp

So the compiler is seeing you define Paddle as a subclass of Entity which hasn't actually been defined yet. You can verify this by having the compiler emit the preprocessed source for Entity.cpp, if you like (although it will be a lot of stuff to wade through).

Since Entity.h doesn't need GameMode.h, remove it. That will break the above chain (although you may have this problem elsewhere and may need to perform a similar analysis and correction elsewhere).

 

Share this post


Link to post
Share on other sites

Thanks for the answers guys, that totally fixed it!

Also double thanks for the explanation @jpetrie which clarified things a bit, though even with that I can see that I still can't fully wrap my head around it, so I'll try and search for more explanations(but with images, I always need images to easily understand :D )

Share this post


Link to post
Share on other sites

Make a new text and copy Entity.cpp into it. Then, as long as there is any #include "YourFile.h" left in the text:

  • Open YourFile.h.
  • If YourFile.h contains #pragma once and you've already seen YourFile.h, stop.
  • Otherwise, copy all of YourFile.h.
  • Paste it into the text file, replacing the original #include. 

You can skip the SDL and C++ library includes for sanity. Once you've done that for every #include of your files, read the resulting source code. This is what the compiler is seeing, and it should be clear why this is an issue then.

Share this post


Link to post
Share on other sites

Ok, maybe I am getting it, so the problem is not that Entity.h is not included, the problem is that it appears below the content of Paddle.h

Does this means I could have solved the problem above also by forward declaring class Entity; inside of Paddle.h? :S

 

Edited by MarcusAseth

Share this post


Link to post
Share on other sites
26 minutes ago, MarcusAseth said:

Does this means I could have solved the problem above also by forward declaring class Entity; inside of Paddle.h? :S

 

Not in this case, because Paddle.h declares Paddle which is a subclass of Entity, and inheriting from some type requires the full definition of that type. A forward-declaration won't suffice.

Share this post


Link to post
Share on other sites

You can avoid passing GameMode* gameRef as a parameter to the entity, and paddle, etc. by using a class that gets the data from its list of entities, and its reference to GameMode, then does whatever you have entity doing with it.

Then you shouldn't have to include GameMode.h at all in Entity.h or Paddle.h or their .cpp files. Also, it doesn't look like you need to include Entity.h or Paddle.h in GameMode.h or even declare the classes, just include them in GameMode.cpp if needed, which you probably shouldn't need to.

If GameMode required a pointer to Entity or Paddle this would create a problem with them having references to each other, using shared_ptr or not handling them properly as they are now with a reference counting system, neither one would be released/deleted when the game closed resulting in a memory leak.

Share this post


Link to post
Share on other sites
23 minutes ago, Yxjmir said:

You can avoid passing GameMode* gameRef as a parameter to the entity, and paddle, etc. by using a class that gets the data from its list of entities, and its reference to GameMode, then does whatever you have entity doing with it.

I don't understand this x_x

Quote

If GameMode required a pointer to Entity or Paddle this would create a problem with them having references to each other, using shared_ptr or not handling them properly as they are now with a reference counting system, neither one would be released/deleted when the game closed resulting in a memory leak.

and this as well x_x

Share this post


Link to post
Share on other sites

Entity.h

Update Paddle.h the same way, by removing include "GameMode.h", and other things related to GameMode

#pragma once
#include "SDL2\SDL.h"
#include "Utility.h"
using namespace util;

enum class PivotMode: Uint8 {CENTER,TOP_LEFT};

class Entity
{
protected://variables
	float XCenter;
	float YCenter;
	SDL_Rect CollisionBox;
	SDL_Texture* Sprite;
	SDL_Renderer* Renderer;	// This can also be moved outside of this class if needed

public://constructors
	Entity(PivotMode inputMode, int x, int y, std::string path);
	virtual ~Entity();
	Entity(const Entity&) = delete;
	Entity& operator=(const Entity&) = delete;
	Entity(Entity&&) = delete;
	Entity& operator=(Entity&&) = delete;

public://methods
	virtual void Update(float deltaTime) = 0;
	virtual void Draw(float interpolation) = 0;

private://methods
	void SetTexture(SDL_Texture* texture)const; // Call this from the new class below to set the enitity's texture
	void SetCollisionBox(int x, int y, PivotMode InputMode);
};

GameMode.h

Removed #include "Entity.h" and #include "Paddle.h", also removed class declarations:

class Entity;

class Paddle;

#pragma once
#include "SDL2\SDL.h"
#include <string>
#include <vector>
#include <memory>
#include "App.h"

class GameMode
{
	friend class App;
private://variables
	bool Running;
	SDL_Window* Window;
	SDL_Renderer* Renderer;
	App* AppRef;

public://constructors
	GameMode(SDL_Window* Window, SDL_Renderer* Renderer, App* App);
	~GameMode();

	GameMode(const GameMode&) = delete;
	GameMode& operator=(const GameMode&) = delete;	
	GameMode(GameMode&&) = delete;
	GameMode& operator=(GameMode&&) = delete;

public://methods
	SDL_Texture* RequestTexture(std::string path)const;
	bool IsRunning()const;
	SDL_Renderer* GetRenderer()const;

private://methods
	void Update(float deltaTime);
	void Draw(float interpolation);
};

ClassName.h

#include "Paddle.h"
#include "GameMode.h"

class ClassName
{
	GameMode* m_pGameMode;
	std::list<Entity*> m_pEntities; // List of all entities, does not have to include paddle if you include the following line
	// but you have make sure you update and render the paddle and this list.
	
	Paddle* m_pPaddle;	// I've only included this for easier access to the paddle

	// Other class variables/functions
}

ClassName.cpp

ClassName::ClassName(GameMode* gameRef) : m_pGameMode{ gameRef }
{
	if (m_pGameMode) { Renderer = m_pGameMode->GetRenderer(); }
	// You can comment this out, and create and initialize m_pGameMode here, but its not necessary
}

ClassName::~ClassName()
{
}

SDL_Texture* ClassName::RequestTexture(std::string path)const
{
	if (m_pGameMode->IsRunning())
	{
		return m_pGameMode->RequestTexture(path);
	}
	return nullptr;
}

// Other Funtions

You probably won't have to worry about the last part of my previous post, but if you started using shared_ptr or something similar, then you'd have a potential memory leak. Basically, two classes would not let go of their references to each other until the other one did, so they never would, I don't know how to put it simpler than that, so if you still don't understand, then look it up.

Share this post


Link to post
Share on other sites

So the benefit of doing that would be that I store only 1 pointer to GameMode (as opposed to 1 per entity) and I pass 1 less argument at Entity construction, right?

I don't know, doesn't seems worth adding another layer of complexity, currently GameMode is holding the vector<unique_ptr<Entity>> so I could instead assign the GameMode "this" to a global GameMode* during GameMode construction so that every Entity can see and use it.

Unless I am missing some other benefit, doesn't seems worth adding one more layer :/

Edited by MarcusAseth

Share this post


Link to post
Share on other sites

I didn't see that vector<unique_ptr<Entity>> in GameMode.h, which means the class isn't actually holding onto it, unless you left it out in your post.

You can just use GameMode the same way as my "class ClassName" example. This will require you to include Paddle.h in GameMode.h or GameMode.cpp if the entity list is stored there. You still won't need the forward declarations for those Entity or Paddle in GameMode.h regardless. And Neither Paddle.h or Entity.h (or their cpp files) should include GameMode.h.

32 minutes ago, MarcusAseth said:

so I could instead assign the GameMode "this" to a global GameMode* during GameMode construction so that every Entity can see and use it.

Yes you could, but Entity/Paddle don't need to have access to GameMode or use data from it. GameMode already has access to them, and should be able to manipulate/set their data as it needs to.

I was actually going to suggest this first, but a lot of people don't like having even one global variable at all, so I figured I'd offer another solution. There's nothing wrong with this, as long as you don't start doing it for all of you classes.

I hope this makes sense.

Edited by Yxjmir
typo

Share this post


Link to post
Share on other sites
44 minutes ago, Yxjmir said:

I didn't see that vector<unique_ptr<Entity>> in GameMode.h, which means the class isn't actually holding onto it, unless you left it out in your post.

My bad, is a modify I made after this topic was posted. Though I don't think I can remove #include GameMode.h from the cpp files of the entities because they need more than a forward declare in order to call methods on GameMode :\

Share this post


Link to post
Share on other sites
13 hours ago, MarcusAseth said:

I don't think I can remove #include GameMode.h from the cpp files of the entities because they need more than a forward declare in order to call methods on GameMode :\

I think you're missing the point. GameMode can send the texture, and whatever else it needs to, to the Entity when the Entity or unique_ptr<Entity> is created. All you have to do is make "SDL_Texture* Sprite;" a public variable in the Entity class so GameMode can access it, or create a "SetTexture(SDL_Texture* sprite)" function that GameMode calls and supplies with the texture as a parameter.

The only other thing that I saw that might require GameMode.h to be included in Entity.cpp is SDL_Renderer* Renderer; but you already have it in the GameMode class, so Entity doesn't need to store it.

class GameMode
{
	friend class App;
private://variables
	bool Running;
	SDL_Window* Window;
	SDL_Renderer* Renderer;	// <-- Just send this to each Entity during Entity::Draw if needed
	App* AppRef;

public://constructors
	GameMode(SDL_Window* Window, SDL_Renderer* Renderer, App* App);
	~GameMode();

Its up to you whether or not to do this, I'm just trying to explain it so you can see how its possible.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now


  • Forum Statistics

    • Total Topics
      628702
    • Total Posts
      2984299
  • Similar Content

    • By mister345
      Hi, I'm building a game engine using DirectX11 in c++.
      I need a basic physics engine to handle collisions and motion, and no time to write my own.
      What is the easiest solution for this? Bullet and PhysX both seem too complicated and would still require writing my own wrapper classes, it seems. 
      I found this thing called PAL - physics abstraction layer that can support bullet, physx, etc, but it's so old and no info on how to download or install it.
      The simpler the better. Please let me know, thanks!
    • By lawnjelly
      It comes that time again when I try and get my PC build working on Android via Android Studio. All was going swimmingly, it ran in the emulator fine, but on my first actual test device (Google Nexus 7 2012 tablet (32 bit ARM Cortex-A9, ARM v7A architecture)) I was getting a 'SIGBUS illegal alignment' crash.
      My little research has indicated that while x86 is fine with loading 16 / 32 / 64 bit values from any byte address in memory, the earlier ARM chips may need data to be aligned to the data size. This isn't a massive problem, and I see the reason for it (probably faster, like SIMD aligned loads, and simpler for the CPU). I probably have quite a few of these, particular in my own byte packed file formats. I can adjust the exporter / formats so that they are using the required alignment.
      Just to confirm, if anyone knows this, is it all 16 / 32 / 64 bit accesses that need to be data size aligned on early android devices? Or e.g. just 64 bit size access? 
      And is there any easy way to get the compiler to spit out some kind of useful information as to the alignment of each member of a struct / class, so I can quickly pin down the culprits?
      The ARM docs (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html) suggest another alternative is using a __packed qualifier. Anyone used this, is this practical?
    • By Josheir
      In the following code:

       
      Point p = a[1]; center of rotation for (int i = 0; I<4; i++) { int x = a[i].x - p.x; int y = a[i].y - p.y; a[i].x = y + p.x; a[i].y = - x + p.y; }  
      I am understanding that a 90 degree shift results in a change like:   
      xNew = -y
      yNew = x
       
      Could someone please explain how the two additions and subtractions of the p.x and p.y works?
       
      Thank you,
      Josheir
    • By alex1997
      Hey, I've a minor problem that prevents me from moving forward with development and looking to find a way that could solve it. Overall, I'm having a sf::VertexArray object and looking to reander a shader inside its area. The problem is that the shader takes the window as canvas and only becomes visible in the object range which is not what I'm looking for.. 
      Here's a stackoverflow links that shows the expected behaviour image. Any tips or help is really appreciated. I would have accepted that answer, but currently it does not work with #version 330 ...
    • By noodleBowl
      I just finished up my 1st iteration of my sprite renderer and I'm sort of questioning its performance.
      Currently, I am trying to render 10K worth of 64x64 textured sprites in a 800x600 window. These sprites all using the same texture, vertex shader, and pixel shader. There is basically no state changes. The sprite renderer itself is dynamic using the D3D11_MAP_WRITE_NO_OVERWRITE then D3D11_MAP_WRITE_DISCARD when the vertex buffer is full. The buffer is large enough to hold all 10K sprites and execute them in a single draw call. Cutting the buffer size down to only being able to fit 1000 sprites before a draw call is executed does not seem to matter / improve performance.  When I clock the time it takes to complete the render method for my sprite renderer (the only renderer that is running) I'm getting about 40ms. Aside from trying to adjust the size of the vertex buffer, I have tried using 1x1 texture and making the window smaller (640x480) as quick and dirty check to see if the GPU was the bottleneck, but I still get 40ms with both of those cases. 

      I'm kind of at a loss. What are some of the ways that I could figure out where my bottleneck is?
      I feel like only being able to render 10K sprites is really low, but I'm not sure. I'm not sure if I coded a poor renderer and there is a bottleneck somewhere or I'm being limited by my hardware

      Just some other info:
      Dev PC specs: GPU: Intel HD Graphics 4600 / Nvidia GTX 850M (Nvidia is set to be the preferred GPU in the Nvida control panel. Vsync is set to off) CPU: Intel Core i7-4710HQ @ 2.5GHz Renderer:
      //The renderer has a working depth buffer //Sprites have matrices that are precomputed. These pretransformed vertices are placed into the buffer Matrix4 model = sprite->getModelMatrix(); verts[0].position = model * verts[0].position; verts[1].position = model * verts[1].position; verts[2].position = model * verts[2].position; verts[3].position = model * verts[3].position; verts[4].position = model * verts[4].position; verts[5].position = model * verts[5].position; //Vertex buffer is flaged for dynamic use vertexBuffer = BufferModule::createVertexBuffer(D3D11_USAGE_DYNAMIC, D3D11_CPU_ACCESS_WRITE, sizeof(SpriteVertex) * MAX_VERTEX_COUNT_FOR_BUFFER); //The vertex buffer is mapped to when adding a sprite to the buffer //vertexBufferMapType could be D3D11_MAP_WRITE_NO_OVERWRITE or D3D11_MAP_WRITE_DISCARD depending on the data already in the vertex buffer D3D11_MAPPED_SUBRESOURCE resource = vertexBuffer->map(vertexBufferMapType); memcpy(((SpriteVertex*)resource.pData) + vertexCountInBuffer, verts, BYTES_PER_SPRITE); vertexBuffer->unmap(); //The constant buffer used for the MVP matrix is updated once per draw call D3D11_MAPPED_SUBRESOURCE resource = mvpConstBuffer->map(D3D11_MAP_WRITE_DISCARD); memcpy(resource.pData, projectionMatrix.getData(), sizeof(Matrix4)); mvpConstBuffer->unmap(); Vertex / Pixel Shader:
      cbuffer mvpBuffer : register(b0) { matrix mvp; } struct VertexInput { float4 position : POSITION; float2 texCoords : TEXCOORD0; float4 color : COLOR; }; struct PixelInput { float4 position : SV_POSITION; float2 texCoords : TEXCOORD0; float4 color : COLOR; }; PixelInput VSMain(VertexInput input) { input.position.w = 1.0f; PixelInput output; output.position = mul(mvp, input.position); output.texCoords = input.texCoords; output.color = input.color; return output; } Texture2D shaderTexture; SamplerState samplerType; float4 PSMain(PixelInput input) : SV_TARGET { float4 textureColor = shaderTexture.Sample(samplerType, input.texCoords); return textureColor; }  
      If anymore info is needed feel free to ask, I would really like to know how I can improve this assuming I'm not hardware limited
  • Popular Now