Sign in to follow this  
Last Hylian

SDL blit troubles

Recommended Posts

Alright, so, yeah, just trying my hand at SDL, and I know how to Blit surfaces, but I'm trying to right a functions to streamline it all. For some reason though, the image isn't showing up, so, if someone would like to enlighten me, that'd be great. main.cpp
#include "sdl.h"
#include "main_functions.h"

int main(int argc, char* argv[])
{
	main_functions mainfunct;

	mainfunct.execute();

	return 0;
}
main_functions.h
#ifndef MAIN_FUNCTIONS_H_
#define MAIN_FUNCTIONS_H_
#include "sdl.h"
#include "graphic_functions.h"

class main_functions
{
private:
	bool running;
	SDL_Event Event;
public:
	SDL_Surface* screen;
public:
	main_functions();
	void execute();
	bool start();
	void check_event(SDL_Event Event);
	void draw();
	void clean_up();
};

#endif
main_functions.cpp
#include "main_functions.h"

main_functions::main_functions()
{
	running = true;
	screen = NULL;
}

bool main_functions::start()
{
	if((SDL_Init(SDL_INIT_EVERYTHING)) < 0)
	{
		return false;
	}
	if((screen = SDL_SetVideoMode(600,400,16, SDL_HWSURFACE | SDL_DOUBLEBUF)) == NULL)
	{
		return false;
	}

	return true;
}

void main_functions::check_event(SDL_Event Event)
{
	graphic_functions graphfunct;
	if(SDL_PollEvent(&Event))
	{
		if(Event.type == SDL_KEYDOWN)
		{
		switch(Event.key.keysym.sym)
			{
			case SDLK_UP: graphfunct.subtracty();break;
			case SDLK_DOWN: graphfunct.addy();break;
			case SDLK_LEFT: graphfunct.subtractx(); break;
			case SDLK_RIGHT: graphfunct.addx();break;
			}
		}
		else if(Event.type == SDL_QUIT)
		{
			running = false;
		}
	}
}

void main_functions::draw()
{
	graphic_functions graphfunct;

	graphfunct.prepare_image("avy.bmp");
	graphfunct.draw_sprite(graphfunct.avy,graphfunct.getx(),graphfunct.gety());
}

void main_functions::clean_up()
{
	SDL_FreeSurface(screen);
}

execute.cpp
#include "main_functions.h"

void main_functions::execute()
{
	start();
	while(running)
	{
		check_event(Event);
		draw();
		SDL_Flip(SDL_GetVideoSurface());
	}
	clean_up();
}
graphic_functions.h
#ifndef GRAPHIC_FUNCTIONS_H_
#define GRAPHIC_FUNCTIONS_H_
#include "sdl.h"
#include "main_functions.h"

class graphic_functions
{
private:
	int playerx;
	int playery;
public:
	//surfaces for optimization
	SDL_Surface* original_image;
	SDL_Surface* optimized_image;

	//actual game graphics
	SDL_Surface* avy;

	graphic_functions();
	void load_image(char* file);
	void prepare_image(char* file);
	void draw_sprite(SDL_Surface* sprite, int x, int y);
	void moveplayer(SDL_Event Event);
	int getx();
	int gety();
	void addx();
	void subtractx();
	void addy();
	void subtracty();
};

#endif
graphic_functions.cpp
#include "graphic_functions.h"

graphic_functions::graphic_functions()
{
	SDL_Surface* original_image = NULL;
	SDL_Surface* optimized_image = NULL;
	SDL_Surface* avy = NULL;
	playerx = 1;
	playery = 1;
}

void graphic_functions::load_image(char* file)
{
	original_image = SDL_LoadBMP(file);
}

void graphic_functions::prepare_image(char* file)
{
	load_image(file);
	optimized_image = SDL_DisplayFormat(original_image);
	avy = optimized_image;
}

void graphic_functions::draw_sprite(SDL_Surface* sprite, int x, int y)
{
	main_functions mainfunct;

	SDL_Rect rect;
	rect.x = x;
	rect.y = y;
	SDL_BlitSurface(sprite,NULL,SDL_GetVideoSurface(),&rect);
}

int graphic_functions::getx()
{
	return playerx;
}

int graphic_functions::gety()
{
	return playery;
}

void graphic_functions::addx()
{
	playerx++;
}

void graphic_functions::addy()
{
	playery++;
}

void graphic_functions::subtractx()
{
	playerx--;
}

void graphic_functions::subtracty()
{
	playery--;
}


So, anybody know what's up? [Edited by - Last Hylian on September 10, 2009 8:12:57 PM]

Share this post


Link to post
Share on other sites
Hm, after taking a closer look at your code (should have noticed this earlier, sorry :s ), it seems you're declaring another main_functions instance in graphic_functions::draw_sprite -- but its SDL_Surface* screen pointer won't be the same as the one used everywhere else (in fact it'll be NULL)
You should probably structure your code a bit differently, but a quick workaround for now would be to use SDL_GetVideoSurface() to get a pointer to the screen rather than trying to access the one held by mainfunct

Share this post


Link to post
Share on other sites
You have scope issues it looks like.

Another thing is that every time you are calling yr draw() method, you are reloading the image into a surface (see yr prepare_image() method). This is not good since every time you are drawing the image you are reloading it from file!

Also, use

[*source][*/source] tags when posting code.
Just remove the *'s in the tags. Quote this post to see how or read FAQ.

[Edited by - signal_ on September 9, 2009 10:39:12 PM]

Share this post


Link to post
Share on other sites
Alright, I'll fix those tags in the original post. Ok, and, sorry, but I have one more question. I figured better to ask it here than start a new thread. Again, I'm quite new to this, so I'm there are a few I'm trying a few basic things and, though it's not returning any errors, it's just not working. Because of that, I really have nothing I can do but ask for help. I'll update the original post. Now I'm working on being able to move the image I just blitted around the screen, I think my logic is sound, but apparently it isn't adequate. I didn't get this from a tutorial, I came up with this method on my own, so I figured I'd probably have to iron out a few posts. Check the original post and see if you see any faults in the code or in the logic behind the code, please. Thanks.

Share this post


Link to post
Share on other sites
Similar problem -- you're declaring new instances of graphic_functions in both main_functions::check_event and in main_functions::draw. Not only are those not the same instance (so the x and y won't be the same between function calls), but you're declaring them on the stack (i.e. without using new), so they're local to the functions and get deallocated when they go out of scope.
Again, you should probably structure your code a bit differently -- for instance, you can have an instance of graphic_functions be a member of main_functions.

As for your movement logic, it's mostly okay, but notice that you only change the value of x and y when the user presses a key -- if the user holds a key down, the sprite will only move a single pixel. A basic (but helpful) motion tutorial can be found here.

Share this post


Link to post
Share on other sites
By having an instance of graphic_functions be a member of main_functions, you mean that I should declare that instance along with the functions of the main_function class, right? Because, I'm trying that and seem to be getting an error, so, before I went into the process of trying to fix the error, I wanted to make sure that's what you were advising I do.

Share this post


Link to post
Share on other sites
Yes, that's what I meant, but it was just one (probably bad, even) possible suggestion -- how you go about structuring your code is up to you (e.g. you could make playerx and playery static and keep the rest as is, or you could move them out graphic_functions entirely since they probably don't really belong there, or you could come up with a completely new class hierarchy, or whatever)
It's not an SDL problem, so you have really any number of ways to deal with that

Share this post


Link to post
Share on other sites
Quote:
Original post by Last Hylian
By having an instance of graphic_functions be a member of main_functions, you mean that I should declare that instance along with the functions of the main_function class, right? Because, I'm trying that and seem to be getting an error, so, before I went into the process of trying to fix the error, I wanted to make sure that's what you were advising I do.

The problem relates to the fact that graphics_functions includes main_functions.h, and main_functions includes graphics_functions.h. This is a circular dependency, and C++'s prehistoric compilation model cannot handle it (with ease at any rate).

I'm going to show you a few things. The first is to point you towards this article, which you should be able to use to get your program to work. I strongly encourage you to take this step now, because if you skip it now you will come across it later.

The second thing is that I will point out that the dependency is one way. The only refernce to "main_functions" in graphics_functions.cpp is an unnecessary variable declaration. class makes use of the graphics_functions class. This means that we can remove that line and the include from graphics_functions.h and get the program to work.

Finally, I am going to make some stylistic proposals to you. Some of them are subjective, but most have clear advantages over how your code is structured at the moment.

I will use comments in the code to indicate why I made certain changes:

// I'm changing the file name to "graphics.h". Hopefully the
// reason will become clear shortly
#ifndef GRAPHICS_H
#define GRAPHICS_H

// we will be using the C++ standard library string type
#include <string>
#include "SDL.h"

// Here is a class that represents an image in memory.
// Note that the class name begins with an uppercase letter.
// This is a very common (but not quite universal) convention.
//
// Also, I wouldn't include the word "functions" in a class name.
// A class is supposed to represent a thing, a thing with behaviour.
// main_functions sounds more like a namespace.
class Image
{
public:
// You can create an Image by passing a filename
Image(const std::string &file);

// The image will clean up after itself when it goes out of scope
~Image();

// Does exactly what it says on the tin =)
void draw(SDL_Surface *destination, int x, int y);

private:
// note how this variable is private
SDL_Surface *surface;
};

// This class represents the screen which we draw to.
class Screen
{
public:
// The screen allows client code to specify the screen parameters.
// This function should call SDL_Init()
Screen(int width, int height, int bpp, Uint32 flags);

// You can call SDL_Quit() here.
~Screen();

// Two things to note about this function
// * See how our class naming convention comes in handy?
// We can call the unspecified Image "image", rather than thinking of
// some other name for the variable.
//
// * See how the private surface "screen" is protected. We pass the screen
// to functions that need it, like Image::draw() and SDL_Flip()
// but no outside code can touch "screen". This means that if we find a
// bug affecting the value of "screen", the chances are it is one of the
// member functions in the Screen class.
void draw(Image &image, int x, int y)
{
image.draw(screen, x, y);
}

void flip();

private:
SDL_Surface *screen;
};

#endif


I'm sure you can write graphics.cpp without my help =)

// Another name change
#ifndef GAME_H
#define GAME_H

#include "graphics.h"

// The player is going to be an instance of a more general class, the sprite
// You can have many sprites with different image files.
// Sprites have a position and a velocity.
class Sprite
{
public:
// This next bit might be new to you.
// This syntax is called an "initialiser list".
// It is used in a constructor to call the constructors of members
// or superclasses.
Sprite(const std::string &imageFile) : image(imageFile), x(0), y(0)
{
}

// Call this every frame to update the sprites position.
void update();

// Set the sprite's speed using these functions.
void setSpeedX(int amount);
void setSpeedY(int amount);

// Another Ronseal function =)
void draw(Screen &screen);
private:
Image image;
int x, y;
int vx, vy;
};

// Wraps up playing a game.
class Game
{
public:
Game();

void run();
private:
Screen screen;
Sprite player;
};

#endif


I think you can imagine how main.cpp would look. Its very similar to your current main file.

However, I will show some of the functions from game.cpp:

#include "game.h"

void Sprite::update()
{
x += vx;
y += vy;
}

// I think you can handle setSpeed*() =)

void Sprite::draw(Screen &screen)
{
screen.draw(image, x, y);
}

// Our "Game" constructor must initialise the screen:
Game::Game() : Screen(600, 400, 16, SDL_HWSURFACE | SDL_DOUBLEBUF)
{
}

/*
These next two functions are helper functions, to simplify the logic
of Game::run(). However, because they are not intended to be used by client
code, they are not exposed in the public interface in game.h.

Instead, the game instance passes the state it needs to these functions.
Here, they only need access to the player instance, to move it around.

This is in contrast to your old interface for main_functions, which
exposed a lot of unnecessary functionality to client code.

Ideally, these helper functions would go in an anonymous namespace, but
I think that is a lesson for another day.
*/


void handleKeyPress(Player &player, const SDL_Event &event)
{
// if its a keydown event, we want the player to move at speed 1.
// otherwise we want to stop the player
int amount = (event.type == SDL_KEYDOWN) ? 1 : 0;

switch(event.key.keysym.sym)
{
case SDLK_UP:
player.setSpeedY(amount);
break;
case SDLK_DOWN:
player.setSpeedY(-amount);
break;
case SDLK_RIGHT:
player.setSpeedX(amount);
break;
case SDLK_LEFT:
player.setSpeedX(-amount);
break;
default:
break;
}
}

// This function returns true if the game should continue.
// It returns false otherwise.
bool handleEvents(Player &player)
{
SDL_Event event;
while(SDL_PollEvent(&event))
{
switch(event.type)
{
case SDL_KEYDOWN:
handleKeyPress(player, event);
break;
case SDL_QUIT:
return false;
break;
default:
break;
}
}
return true;
}

void Game::run()
{
bool running = true;

while(running)
{
sprite.update();
sprite.draw(screen);
screen.flip();
running = handleEvents(player);
}
}


The above isn't perfect, but I think its a good starting point. You might consider moving separate classes to their own files, but that is a minor point. The main thing is to see

Share this post


Link to post
Share on other sites
Quote:
The main thing is to see


Hmm, I suppose he didn't think the main thing was important enough to add to the end of the post =)

Alright, a few things in there I'm unfamiliar with.

Such as:


void Sprite::draw(Screen &screen)
{
screen.draw(image, x, y);
}




Up there, where you put "Screen &screen", are you declaring an instance? I'm unfamiliar with that method. . .

also:


// Our "Game" constructor must initialise the screen:
Game::Game() : Screen(600, 400, 16, SDL_HWSURFACE | SDL_DOUBLEBUF)
{
}




Is this the equivalent of saying "screen = SetVideoMode(ect. . )? I'm also unfamiliar with that method. Like I said, I'm new to SDL.

And last but not least:


Image(const std::string &file);



What do you mean this "creates a new image"? Creates a new image as what, a surface? Forgive my newbiness, but please explain this to me.

[Edited by - Last Hylian on September 11, 2009 4:39:23 PM]

Share this post


Link to post
Share on other sites
Quote:

Hmm, I suppose he didn't think the main thing was important enough to add to the end of the post =)

Sorry, I probably thought of some issue with the code that required attention, then I just forgot to make whatever point I was building to. Given that I can't remember it now, it probably wasn't all that important. [grin]


void Sprite::draw(Screen &screen)
{
screen.draw(image, x, y);
}

In this code, "screen" is a reference. This means that it is an alias for an existing Screen instance, not a copy of some other one. We want to draw a sprite to a particular screen, not create a new screen instance, draw to that and then forget the instance (remember, temporaries are destructed at the end of their scope).

Quote:


// Our "Game" constructor must initialise the screen:
Game::Game() : Screen(600, 400, 16, SDL_HWSURFACE | SDL_DOUBLEBUF)
{
}

Is this the equivalent of saying "screen = SetVideoMode(ect. . )? I'm also unfamiliar with that method. Like I said, I'm new to SDL.

I must point out that I did not compile or test the code. In this case, I made a small typo, in the above it should be "screen", not "Screen". But here what we are doing is initialising the member "screen".

This technique has nothing to do with SDL. It is called an initialiser list. It allows us to specify which values are passed to the constructor for "screen". Note that Screen, like Image, lacks a default constructor (one that can be called without arguments) - so we must provide these values somewhere.

It is related to code like this:

int main()
{
// A new "Screen" instance, called "screen", with the given parameters
Screen screen(800, 600, 32, SDL_ANYFORMAT);

// use screen...
}



As for your last question, it might be possible that you have never come across std::string, or maybe the "const" was throwing you. Combine this with the fact that you don't know about references makes it quite hard for you to figure out what is going on.

std::string is a type that represents some text in C++. We prefer to use std::string over char pointers because the latter are very easy to get wrong and an cause crashes, memory corruption and memory leaks.

We pass by reference to avoid making a copy of the string. This is not strictly necessary, but most C++ programmers pass complex types by reference. Finally, we mark the reference as "const", which prevents it being modified. This is very important when you pass a reference for performance reasons - otherwise you might make what you think are local modifications and actually be modifying the string the caller passed in:

void example(std::string &arg)
{
// an accident, we delete the text in "arg"
arg.clear();
}

int main()
{
std::string str = "hello";
example(str);
// oh no, "str" is now empty!
// had we marked "arg" as const in example(), the compiler would not have
// allowed us to make the mistake.
}


But to simplify, the Image() constructor takes a filename telling it which image to load. Like "screen", this forces all code wanting to make use of an image to specify a filename. This means you cannot have an Image that doesn't represent an image in memory.

This gets me on to a point I did mean to put in but I forgot. When a constructor fails, we cannot use return codes to detect this condition - constructors don't have return types. Instead, we can use exceptions to signal the error.

For example, the Image constructor might look like this:

// Standard C++ exceptions header
#include <stdexcept>

Image::Image(const std::string &filename)
{
surface = SDL_LoadBMP(filename.c_str());

if(!surface)
{
throw std::runtime_error(SDL_GetError());
}

SDL_Surface *optimised = SDL_DisplayFormat(surface);
if(optimised)
{
SDL_FreeSurface(surface);
surface = optimised;
}
}


Callers can detect the error using a try ... catch block.

Share this post


Link to post
Share on other sites
Huh. . . I didn't know a constructor could be used as an actual function. Alright, thanks for explaining those things. I'm still not quite sure I understand all of them, but that's probably just my own ignorance hindering me. I guess I'll have to do a bit more research. Any suggested tutorials/articles rip-off? You've been a great help so far, thanks.

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

Sign in to follow this