0

I'm writting a simple game and I've had it finished, except it had a bad file structure and every class was just an .h file.

I've decided to split the declarations and definitions into separate files, because I ran into circular inclusion issue (which was predictable).

Now that I've split them into separate files, my .h files contains:

  • declaration of class, it's methods, enum and other variables.
  • inclusion of neccessary files that .cpp or .h requires (so basically .cpp includes it's .h file and it get's all the inclusions it needs).

While my .cpp file contains:

  • definition of methods.

Okay, now that you have a general idea on how my files are structured, here's the issue I'm having.

Spaceship.cpp requires cursorPos variable that exists in Core.h

Core.h includes Spaceship.h because it requires it for a method.

If I include Core.h inside Spaceship.h, I get double-declaration.

If I don't include Core.h inside Spaceship.h, I get no-declaration.

I'm clueless, should I create a 'Helper' class that contains variables I want to cross-share between classes?

EDIT Code as requested

Spaceship.h

#pragma once
#ifndef SPACESHIP_H
#define SPACESHIP_H

#include "Vector2D.h"
#include "Entity.h"
#include "Missile.h"
#include <SDL.h>
#include "Core.h"

extern const int SPACESHIP_SHOOT_DELAY = 50;
extern const int SPACESHIP_MAX_VELOCITY = 10;
extern const float SPACESHIP_VELOCITY_GAIN = 0.05;
extern const float SPACESHIP_VELOCITY_LOSS = -0.15;
class Spaceship : public Entity
{
    public:
        Vector2D position = Vector2D(0, 0);
        Vector2D direction = Vector2D(0, 0);
        SDL_Texture* texture;

        //Required by rendering
        SDL_Rect renderbox = { 0, 0, 32, 32 };
        SDL_Rect boundingbox = { 0, 0, 32, 32 };
        SDL_Point center = { 16, 16 };

        int lastShot = SDL_GetTicks();
        int angle = 0;
        float velocity = 0;
        void Update() override;
        void Render(SDL_Renderer* renderer) override;
        void Fire();

        Spaceship();
        ~Spaceship();
};
#endif

Core.h

#pragma once
#ifndef CORE_H
#define CORE_H

#include "Vector2D.h"
#include <SDL.h>
#include "Spaceship.h"

extern Vector2D cursorPos = Vector2D();
class Core
{
    private:
        static bool run;

    public:
        static SDL_Window* window;
        static SDL_Renderer* renderer;
        static SDL_Cursor* cursor;
        static int screenWidth;
        static int screenHeight;

        static void Initialize();
        static void ProcessEvents(SDL_Event* e);
        static void Update();
        static void Render();
        static int Execute();
        static void Cleanup();

        Core();
        ~Core();
};

#endif

And the errors I'm getting enter image description here

3 Answers3

1

Definitions of variables also should be located in .cpp files. Headers should declare it with extern keyword:

Core.h:

extern Point cursorPos;

Core.cpp:

#include "Core.h"
...
Point cursorPos;
Zefick
  • 2,014
  • 15
  • 19
  • That generates even more errors, quadruple declaration in certain cases. Am I missing something? –  Nov 23 '17 at 13:45
  • 2
    It would be a good answer if it insisted on the fact that global variables are _evil_. – YSC Nov 23 '17 at 13:46
  • Everyone says it, but I need to access cursorPos and I'm a freak that hates to have two variables in two different places that do the exact same thing. + I need to access MAX_SHIP_VELOCITY from Spaceship.h and adding extern pops quadruple declaration error. –  Nov 23 '17 at 13:49
  • 1
    @Netheous probably you did something wrong. Generally you can write `extern` declaration any number of times and it will not cause an error: https://ideone.com/63ixwV. – Zefick Nov 23 '17 at 13:49
  • @Zefick just noticed, I had circular inclusion - Core.h -> Spaceship.h, Spaceship.h -> Core.h. But now that I remove include Core.h from Spaceship.h - cursorPos is undeclared. The only solution I can think of is adding a middleman, a Helper class that has these variables and both Core.h and Spaceship.h including it. –  Nov 23 '17 at 13:52
  • So what do you guys suggest, having a file that has all these constants? like SPACESHIP_MAX_VELOCITY? –  Nov 23 '17 at 13:56
  • @Netheous whatever you think you gain by having fewer instances of variables, it is not worth it. A Point variable does not take up a lot of memory, micro optimizations like this, is not something that you will benefit from. Keep variable delaration out of header files, and if you must have a single global instance, use an `extern` defined variable in a header, and declare it in a single source file. Like suggested here. Remember the basics and include the header guards as suggested by another answer. – Tommy Andersen Nov 23 '17 at 14:37
0

Put #pragma once as the top line of every header file. This resolves the multiple-times-included issue (i.e. it will ensure that the compiler will not re-include the code of an include file which it has already included before)

Smeeheey
  • 9,906
  • 23
  • 39
  • 1
    Stricly speaking `#pragma once` is not standard C++... Personally I would prefer a regular header: `#ifndef SOMETHING_H_` `#define SOMETHING_H_` `...` `#endif` – Tommy Andersen Nov 23 '17 at 13:35
  • 1
    Even with `#pragma once` you will get a linkage error if you imclude the header from different cpp-s. Variables always should be declared with `extern`. – Zefick Nov 23 '17 at 13:42
  • 1
    @Tommy Anderson Strictly speaking, names containing double underscores are reserved for the implementation ;). But I agree, use good header guards, which are portable. – oisyn Nov 23 '17 at 13:42
  • @oisyn Strictly speaking "double underscore" is two consecutive underscores, not like in my example. See §5.10.3.1 ;-) – Tommy Andersen Nov 23 '17 at 13:49
  • That's what I meant. It must be my mobile phone's font rendering then, because those underscores look awfully long :). Mea culpa. – oisyn Nov 23 '17 at 13:54
  • @TommyAndersen I've added the inclusion guards, but it pops multum of errors regarding variables being already defined for other variables that have born extern before them and inclusion guards added. –  Nov 23 '17 at 14:21
  • @Netheous it is difficult to answer, without seeing your code, perhaps you could add some code to your question, so it will be easier to answer. – Tommy Andersen Nov 23 '17 at 14:38
0

The use of global variables is an anti-pattern, and for good reasons. However describing all the many reasons why one should avoid global state, is a topic for another day. In your case, you are having trouble with your specific implementation of the global variables. While I would strongly recommend that you revisit your design, and lower the amount of global state, I will try to present a solution and explanation to the problems you are facing.

One problem is understanding the include mechanics of C++, when you include a file in C++ you are basically inserting the contents of that file, in that place in your code. So if you have anything declared in a header file, it will be declared where it is included. If you include a file multiple times, you will insert it multiple times, and thereby declare your variables multiple times. As an example consider my header variables.h:

int someGlobalVariable = 100;

Now consider a second header file: functionality.h, that looks like this:

#include "variables.h"

void foo(int number);

And finally a source file called functionality.cpp:

#include "variables.h"
#include "functionality.h"

void foo(int number) {
    // Do something
}

This will give a compiler error, because our variable someGlobalVariable is defined twice. But why is it that? If we try to do the inclusion we will see why:

// variables.h
int someGlobalVariable = 100;

// functionality.h
int someGlobalVariable = 100;  // <-- included from functionality.h

void foo(int number);

void foo(int number) {
    // Do something
}

As you can see the variable is inserted twice, and that of course is not good, the compile will complain, and for good reasons. The forward declaration of foo is entirely legal though.

We could ofcourse add include guards to our header files:

The file variables.h:

#ifndef VARIABLES_H_
#define VARIABLES_H_

int someGlobalVariable = 100;

#endif

The file functionality.h:

#ifndef FUNCTIONALITY_H_
#define FUNCTIONALITY_H_
#include "variables.h"
#include "functionality.h"

void foo(int number) {
    // Do something
}

#endif

This would cause the someGlobalVariable variable to only be included once in our .cpp source file. However this does not make it truly global. Remember since C++ inserts the contents of the file, at the location that we include the file, we will still declare the variable once for each source independent source file that includes it.

Instead of a global we would have a new instance, for every source module, which is not what you want. You need to ensure that the variable is declared once only, and then referred to. This means that we need to declare the variable in a file that is not included (or at most included once). Let us put it in a .cpp file instead.

So a .cpp file named globals.cpp:

int someGlobalVariable.cpp = 100;

The variable.h file now needs to be modified to read:

#ifndef VARIABLES_H_
#define VARIABLES_H_

extern int someGlobalVariable;

#endif

The extern keyword instructs the linker that the variable is declared in another source unit. So the code after the include guards have been added will look like this, when we insert the includes:

// variables.h
extern int someGlobalVariable;

// functionality.h
// Since VARIABLES_H_ is now defined we will not insert the contents.

void foo(int number);

void foo(int number) {
    // Do something
}

The linker will then know to look for the declaration of someGlobalVariable in another object file. Most importantly all source units that refer to the extern int someGlobalVariable will share the same instance.

In your case you should remove the = from the declaration, the variable should be declared in a source unit of its own. So your line:

extern Vector2D cursorPos = Vector2D();

Becomes:

extern Vector2D cursorPos;

And then in a source (.cpp) file you declare the variable like this:

Vector2D cursorPos = Vector2D();
Tommy Andersen
  • 7,165
  • 1
  • 31
  • 50