3

I really need help on this one cause I am extremely stuck and have no idea what to do.

Edit:

A lot of you guys are saying that I need to use the debugger but let me be clear I have not used C++ for an extremely long time and I've used visual studio for a grand total of 2 weeks so I do not know all the cool stuff it can do with the debugger.

I am a student at university at the beginning of my second year who is trying to work out how to do something mostly by failing.

I AM NOT a professional coder and I don't have all the knowledge that you people have when it comes to these issues and that is why I am asking this question. I am trying my best to show my issue so yes my code contains a lot of errors as I only have a very basic understanding of a lot of C++ principles so can you please keep that in mind when commenting

I'm only posting this here because I can don't know who else to ask right now.

I have a function called world that is suppose to call my render class to draw all the objects inside of its vector to the screen.

#include "C_World.h"

C_World::C_World()
{
// creates an instance of the renderer class to render any drawable objects 
C_Renderer *render = new C_Renderer;

}


C_World::~C_World()
{
delete[] render;
}

// adds an object to the world vector 
void C_World::addToWorld(C_renderable* a) 
 {
world_list.push_back(a);
  }


 void C_World::World_Update()
{
render->ClearScreen();
World_Render();
}


void C_World::World_Render() {

for (int i = 0; i < 1; i++)
{
    //render->DrawSprite(world_list[i]->getTexture(), world_list[i]->get_X,  world_list[i]->get_Y());
    render->DrawSprite(1, 1, 1);
}
}

While testing I commented out the Sprites get functions in order to check if they were causing the issue.

the renderer sprites are added to the vector list in the constructor through the create sprite function

C_Renderer::C_Renderer()
{
// test sprite: Id = 1
CreateSprite("WhiteBlock.png", 250, 250, 1);
}

I thought this might of been the issue so I had it in other functions but this didn't solve anything

Here are the Draw and create Sprite functions

 // Creates a sprite that is stored in the SpriteList
 // Sprites in the spriteList can be used in the drawSprite function
 void C_Renderer::CreateSprite(std::string texture_name,
 unsigned int Texture_Width, unsigned int Texture_height, int spriteId) 
 {
 C_Sprite *a = new C_Sprite(texture_name,Texture_Width,
 Texture_height,spriteId);
 SpriteList.push_back(a);
 size_t b = SpriteList.size();
 HAPI.DebugText(std::to_string(b));
 }

// Draws a sprite to the X and Y co-ordinates
void C_Renderer::DrawSprite(int id,int x,int y)
 {
 Blit(screen, _screenWidth, SpriteList[id]->get_Texture(), 
 SpriteList[id]->getTexture_W(), SpriteList[id]->getTexture_H(), x, y);
 }

I even added some test code into the create sprite function to check to see if the sprite was being added too the vector list. It returns 1 so I assume it is.

Exception thrown: read access violation.
std::_Vector_alloc<std::_Vec_base_types<C_Sprite *,
std::allocator<C_Sprite     *> > >::_Mylast(...) returned 0x8.

that is the full error that I get from the compiler

I'm really really stuck if there is anymore information you need just say and ill post it straight away

Edit 2:

 #pragma once
 #include <HAPI_lib.h>
 #include <vector>
 #include <iostream>
 #include "C_renderable.h"
 #include "C_Renderer.h"
class C_World
{
public:
C_World();
~C_World();
C_Renderer *render = nullptr;
void World_Update();
void addToWorld(C_renderable* a);
private:
std::vector<C_renderable*> world_list;
void C_World::World_Render();


};


#pragma once
#include <HAPI_lib.h>
#include "C_renderable.h"
#include "C_Sprite.h"
#include <vector>

class C_Renderer
{

public:
C_Renderer();
~C_Renderer();
// gets a pointer to the top left of screen
BYTE *screen = HAPI.GetScreenPointer();
void Blit(BYTE *destination, unsigned int destWidth,
BYTE *source, unsigned int sourceWidth, unsigned int sourceHeight,
int posX, int posY);
void C_Renderer::BlitBackground(BYTE *destination,
unsigned int destWidth,   unsigned int destHeight, BYTE *source,
unsigned int sourceWidth, unsigned int   sourceHeight);
void SetPixel(unsigned int x,
unsigned int y, HAPI_TColour col,BYTE *screen,    unsigned int width);
unsigned int _screenWidth = 1750;
void CreateSprite(std::string texture_name, 
unsigned int Texture_Width,unsigned int Texture_height, int spriteId);
void DrawSprite(int id, int x, int y);
void ClearScreen();
private:
std::vector<C_Sprite*> SpriteList;


 };
Machavity
  • 30,841
  • 27
  • 92
  • 100
Nasanooel
  • 43
  • 1
  • 5
  • The right tool to solve such problems is your debugger. You should step through your code line-by-line *before* asking on Stack Overflow. For more help, please read [How to debug small programs (by Eric Lippert)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). At a minimum, you should \[edit] your question to include a [Minimal, Complete, and Verifiable](http://stackoverflow.com/help/mcve) example that reproduces your problem, along with the observations you made in the debugger. – πάντα ῥεῖ Oct 23 '16 at 18:19
  • Just as a guess: You're missing some special members. See [What is the rule of three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) and do not try to use pointers at all. – Pixelchemist Oct 23 '16 at 18:22
  • 1
    Without complete example we can only speculate (see first comment on that). The constructor initializes local variable, not class variable - that's one error. The destructor uses wrong deleter (array while constructor didn't construct array) - that's second error. DrawSprite uses `[]` on SpriteList while it should use `find` or `at` instead (you don't want it to create new entry for invalid id). You really should've used debugger first; I suspect just going few frames up in stack trace would've shown that one of your object pointers was NULL. And also please consider using smart pointers – Andrey Turkin Oct 23 '16 at 18:37
  • I'm new to visual studio and I'm not a professional so I don't know how to do a lot of things so how would I do a stack trace? – Nasanooel Oct 23 '16 at 18:40
  • @Nasanooel don't worry too much about certain comments. This is common in SO, and everybody has to deal with that. So just ignore what is not useful to you and focus on what is useful. Try to refrain from saying that you are a beginner or to explain yourself, it wastes your precious time. Be patient, there are certainly people who try to help... – Ely Oct 23 '16 at 18:53
  • When you do `new` you should match it with `delete`. When you do `new[]` you should match it with `delete[]`. – Some programmer dude Oct 23 '16 at 19:12

1 Answers1

3

I don't say this lightly, but the code you've shown is absolutely terrible. You need to stop and go back several levels in your understanding of C++.

In all likeliness, your crash is the result of a simple "shadowing" issue in one or more of your functions:

C_World::C_World()
{
// creates an instance of the renderer class to render any drawable objects 
C_Renderer *render = new C_Renderer;
}

C_World::~C_World()
{
delete[] render;
}

There are multiple things wrong here, and you don't show the definition of C_World but if this code compiles we can deduce that it has a member render, and you have fallen into a common trap.

C_Renderer *render = new C_Renderer;

Because this line starts with a type this is a definition of a new, local variable, render. Your compiler should be warning you that this shadows the class-scope variable of the same name.

What these lines of code

C_World::C_World()
{
// creates an instance of the renderer class to render any drawable objects 
C_Renderer *render = new C_Renderer;
}

do is:

. assign an undefined value to `this->render`,
. create a *local* variable `render`,
. construct a dynamic `C_Renderer` presumably on the heap,
. assign that to the *local* variable `render`,
. exit the function discarding the value of `render`.

So at this point the memory is no-longer being tracked, it has been leaked, and this->render is pointing to an undefined value.

You repeat this problem in several of your functions, assigning new results to local variables and doing nothing with them. It may not be this specific instance of the issue that's causing the problem.

Your next problem is a mismatch of new/delete vs new[]/delete[]:

C_World::~C_World()
{
delete[] render;
}

this would result in undefined behavior: this->render is undefined, and delete[] on a non-new[] allocation is undefined.

Most programmers use a naming convention that distinguishes a member variable from a local variable. Two common practices are an m_ prefix or an _ suffix for members, e.g.

class C_World
{
public:
     C_Foo* m_foo;  // option a
     C_Renderer* render_;  // option b
// ...
}

Perhaps you should consider using modern C++'s concept of smart pointers:

#include <memory>

class C_World {
    // ...
    std::unique_ptr<C_Renderer> render_;
    // ...
};

C_World::C_World()
    : render_(new C_Renderer)   // initializer list
{}

But it's unclear why you are using a dynamic allocation here in the first place. It seems like an instance member would be better:

class C_World {
    C_Renderer render_;
};

C_World::C_World() : render_() {}
kfsone
  • 23,617
  • 2
  • 42
  • 74
  • I appreciate you attempting to solve my issue even though my code is terrible. I knew that it would appear terrible and that the issue was my own ineptitude but please understand I am still learning how to code in C++ so I really really really appreciate you not just solving the issue but also telling me how to make it better I am sorry my coding skills offended you – Nasanooel Oct 23 '16 at 18:53
  • @Nasanooel No, not offended, quite the opposite. C++ is a loaded-shotgun, ready to trip you up and shoot you at any twist or turn. I happen to know C++, but there are countless other languages that I can't write 'hello world' in and would probably blue-screen a macintosh if I tried to. – kfsone Oct 23 '16 at 18:55
  • @Nasanooel If my first paragraph seems harsh, let me expound: The *code* is terrible, but that's because of some simple misunderstandings that have lead you down a certain path, and I say "go back" rather than "give up". What you did makes sense given the misunderstandings, so I'm confident that if you go back and review the basics it'll transform your understanding of the rest of the language and your entire approach. Good luck! – kfsone Oct 23 '16 at 19:03
  • You have helped a bit though with the delete thing I was just taught that delete and delete[] were essentially the same thing so that is why I used it. and I really should be including the definition of my renderer class and my world class to help people solve my issue – Nasanooel Oct 23 '16 at 19:04