TheNexus's Profile User Rating: -----

Reputation: 1 Apprentice
Group:
New Members
Active Posts:
2 (0 per day)
Joined:
13-May 12
Profile Views:
182
Last Active:
User is offline May 13 2012 01:25 AM
Currently:
Offline

Previous Fields

Dream Kudos:
0
Icon   TheNexus has not set their status

Posts I've Made

  1. In Topic: C++ Tile Engine from Scratch -- Part 2

    Posted 13 May 2012

    View PostRevTorA, on 03 May 2011 - 06:15 PM, said:

    There's a catch however. Since Sprites don't actually contain a copy of the Image, if the Image is destroyed (by leaving its scope for instance), the Sprite won't have anything to draw. We could store a copy of each image for every sprite, but that would be inefficient. However, we need to make sure that any Images we use for sprites remain intact for as long as we need it.

    The solution is to have an Image manager class. This class will hold a vector of Images, and we can use it to add or retrieve Images to use with our Sprites.
    Here is the problem: push_back() on a std::vector may trigger a reallocation of the whole sequence, leading to many copies and invalidations of all references. Consider pre-allocation or a different STL container.


    Tile(sf::Image& image);
    
    Since a Tile doesn't modify the sf::Image, you should pass a reference to const.

    void Draw(int x, int y, sf::RenderWindow* rw);
    
    Why do you use pointers here? rw may not be NULL, so I'd suggest to use a reference. Or pointers everywhere, but stay consistent.

    	sf::Image sprite;
    	sprite.LoadFromFile("sprite1.png");
    
    Maybe you should check if the loading succeeds.

    And the other points mentioned in part 1 apply also here: Avoid header guards like _TILE_H, get rid of the memory leaks by replacing dynamic allocations (new) with automatic objects or smart pointers, and don't define empty constructors or destructors.
  2. In Topic: C++ Tile Engine from Scratch -- Part 1

    Posted 13 May 2012

    Your code contains some mistakes and questionable style I'd like to point out:

    #ifndef _ENGINE_H
    #define _ENGINE_H
    
    Identifiers that begin with underscore and captial letter are reserved for compiler and implementation. Anyway, I would use a header guard that is less likely to collide, e.g. REVTORA_TILE_ENGINE_H.

    #include <SFML\Graphics.hpp>
    
    Prefer slashes instead of backslashes, as they are portable.

    Engine::Engine()
    {
    
    }
    
    Engine::~Engine()
    {
    
    }
    
    Empty constructor and destructor are automatically generated by the compiler. But why don't you move the Init() functionality to the constructor? That's why constructors exist, to initialize objects.

    bool Engine::Init()
    {
    	window = new sf::RenderWindow(sf::VideoMode(800, 600, 32), "RPG");
    
    	if(!window)
    		return false;
    
    	return true;
    }
    
    new without std::nothrow never returns a nullpointer, thus the check is useless. Furthermore, you have a memory leak, because the dynamically allocated memory is never freed. You can just use sf::RenderWindow without a pointer, there's no need for dynamic memory here. If there were, you should still prefer std::unique_ptr.

    Engine* engine = new Engine();
    
    Again, this leads to a memory leak. Is there a reason why you don't use
    Engine engine;
    
    ?

My Information

Member Title:
New D.I.C Head
Age:
Age Unknown
Birthday:
Birthday Unknown
Gender:

Contact Information

E-mail:
Private

Friends

TheNexus hasn't added any friends yet.

Comments

TheNexus has no profile comments yet. Why not say hello?