C++ Multiple Definitions

Question

I'm trying to build these files but it's giving me a multiple definition error.

main.cpp:

#include "SDL/SDL.h"
#include "Core.h"
#include "GameStates.h"
#include "globals.h"

int main(int argc, char** args) 
{
if(core.Initilization(640, 480, 32, SDL_SWSURFACE) == -1)
{
    SDL_Quit();
}

while(core.desiredstate != core.quit)
{
    ::currentstate->EventHandling();
    ::currentstate->Logic();
    core.ChangeState();
    ::currentstate->Render();
    ::currentstate->Update();
}

SDL_FreeSurface(core.screen);
SDL_Quit();

}

Core.cpp:

#include "Core.h"
#include "GameStates.h"
#include "SDL/SDL.h"
#include "Intro.h"
#include "globals.h"
#include <string>

/* Starts SDL subsystems and sets screen attributes */
bool Core::Initilization(int SCREEN_WIDTH, int SCREEN_HEIGHT, int SCREEN_BPP, int FLAGS) 
{
    //starts SDL subsystems, returns false upon error
if(SDL_Init(SDL_INIT_EVERYTHING) == -1)
{
    return false;
}

//The screen
screen = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, SCREEN_BPP, FLAGS);
//Returns false if there was an error
if(screen == NULL)
{
    return false;
}

SDL_WM_SetCaption("Game", NULL);

return true;
}

/* Loads an image and optimizes it */
SDL_Surface* Core::Load(std::string filename)
{
//original loaded image
SDL_Surface* original = SDL_LoadBMP(filename.c_str());
SDL_Surface* optimized = NULL;

if(original != NULL)
{
    //Sets optimized to optimized version of original
    optimized = SDL_DisplayFormat(original);

    SDL_FreeSurface(original);
}

return optimized;
} 

/* Blits surfaces */
void Core::ApplySurface(int x, int y, SDL_Surface* source, SDL_Surface* destination) 
{
//holds the x y coordinates
SDL_Rect location;
location.x = x;
location.y = y;

if(destination != NULL) 
{
    SDL_BlitSurface(source, NULL, destination, &location);
}
}

/* Sets desiredstate to be used in ChangeState(); */
void Core::SetState(int newstate) 
{
if(desiredstate != state_null && desiredstate != quit)
{
    desiredstate = newstate;
}
}

/* Changes the game state */
void Core::ChangeState()
{
    if(desiredstate != state_null && desiredstate != quit)
    {
        //frees old state memory
    delete ::currentstate;

    switch(desiredstate)
    {
        case intro:
            //allocates new state memory
            ::currentstate = new Intro();
        break;
    }

    stateID = desiredstate;
    desiredstate = state_null;
}
}

GameStates.h:

#ifndef GAMESTATES_H
#define GAMESTATES_H

class GameStates 
{
public:
    virtual void EventHandling() = 0;
    virtual void Logic() = 0;
    virtual void Render() = 0;
    virtual void Update() = 0;
};

#endif

Intro.h:

#ifndef INTRO_H
#define INTRO_H
#include "SDL/SDL.h"
#include "GameStates.h"

class Intro : public GameStates
{
    private:
    SDL_Surface* test;
public:
    Intro();
    void EventHandling();
    void Logic();
    void Render();
    void Update();
    ~Intro();
} intro;

#endif

Intro.cpp:

#include "Intro.h"
#include "GameStates.h"
#include "Core.h"
#include "SDL/SDL.h"

Intro::Intro()
{
test = core.Load("test.bmp");
}

void Intro::EventHandling()
{
SDL_Event event;
while(SDL_PollEvent(&event))
{
    switch(event.type)
    {
        case SDL_QUIT:
            core.SetState(core.quit);
        break;
    }
}
}

void Intro::Logic()
{
//to be coded when the program actually builds...
}
void Intro::Render()
{
core.ApplySurface(30, 30, test, core.screen);
}

void Intro::Update()
{
SDL_Flip(core.screen);
}

Intro::~Intro()
{
SDL_FreeSurface(test);
}

globals.h:

#include "GameStates.h"
#include "SDL/SDL.h"

GameStates* currentstate = NULL;

Sorry if the indentation is off; having to put four spaces for it to be seen as a code block messed with it a bit.

Heres the error message:

/tmp/ccWxKsO5.o:(.bss+0x0): multiple definition of `core'
/tmp/cc13Eqmt.o:(.bss+0x0): first defined here
/tmp/ccWxKsO5.o:(.bss+0x20): multiple definition of `currentstate'
/tmp/cc13Eqmt.o:(.bss+0x10): first defined here
/tmp/ccJXxewI.o:(.bss+0x0): multiple definition of `intro'
/tmp/ccWxKsO5.o:(.bss+0x10): first defined here
/tmp/ccJXxewI.o:(.bss+0x10): multiple definition of `core'
/tmp/cc13Eqmt.o:(.bss+0x0): first defined here
collect2: ld returned 1 exit status

Makefile:

OBJS = main.o Intro.o Core.o
CC = g++
DEBUG = -g
CFLAGS = -Wall -c $(DEBUG)
LIBS = -lSDL

game : $(OBJS)
    $(CC) $(CFLAGS) $(OBJS) -o game $(LIBS)

main.o : Core.h GameStates.h globals.h 
    $(CC) $(CFLAGS) main.cpp $(LIBS)

Core.o : Core.h Core.cpp GameStates.h Intro.h globals.h
    $(CC) $(CFLAGS) Core.cpp $(LIBS)

Intro.o : Intro.cpp GameStates.h Core.h
    $(CC) $(CFLAGS) Intro.cpp $(LIBS)

Answer 1

For objects shared between multiple translation units, the rule is: there must be exactly one definition, but you may have multiple declarations.

In practice, this means: put "extern Class object;" in your .h file, and "Class object;" in exactly one of your .CPP files.

For intro, for example, change your Intro.h to:

class Intro : public GameStates
{
  ... // whatever 
};
extern Intro intro;

and add this line to Intro.cpp:

Intro intro;

Similarly for currentstate, in globals.h:

extern GameStates* currentstate;

and in one .CPP (it doesn't matter to the compiler which one):

GateStates* currentstate = NULL;

---

P.s. Your makefile is broken. You pass -c, which means "don't link" to your link step. Try this:

OBJS = main.o Intro.o Core.o
CC = g++
DEBUG = -g
CFLAGS = -Wall $(DEBUG)
LIBS = -lSDL

game : $(OBJS)
    $(CC) $(CFLAGS) $(OBJS) -o game $(LIBS)

main.o : Core.h GameStates.h globals.h 
    $(CC) -c $(CFLAGS) main.cpp

Core.o : Core.h Core.cpp GameStates.h Intro.h globals.h
    $(CC) -c $(CFLAGS) Core.cpp

Intro.o : Intro.cpp GameStates.h Core.h
    $(CC) -c $(CFLAGS) Intro.cpp

Answer 2

The problem isn't your code, it's your build system.

Any sane build system matches the name of the object files to the name of the source files. But you have ccWxKsO5.o and cc13Eqmt.o. What's worse, the build system appears to be trying to link multiple objects generated from the same source (perhaps some were created by an earlier run of the compiler).

tempnam and globbing *.o is not a reasonable way to build C++ programs.

---

Well, there may be some code problems also. But those will be a thousand times easier to find and fix once the object names in the error messages correlate to source files.

Answer 3

put include guards in globals.h make GameStates* declaration extern

//globals.h
#ifndef _MY_GLOBALS_H_
#define _MY_GLOBALS_H_
#include "GameStates.h"
#include "SDL/SDL.h"

extern GameStates* currentstate;
#endif

//Main.cpp

#include "globals.h"

GameStates* currentState = 0;

Answer 4

In globals.h, you must declare currentstate extern. Then create globals.cpp, with the definition (GameStates* currentstate = NULL;). I couldn't find any reference for intro or core in your code, but it's probably the same problem: you can declare global variables as often as you want as long as you declare them extern, and only define them once per resulting binary, in only one translation unit.

Also, you probably want to add a header guard (#ifndef GLOBALS_H ...) to globals.h, just in case you add anything else in there.