Read access violation

The Partridge Family were neither partridges nor a family. Discuss.
Post Reply
Calert
Posts: 3
Joined: April 24th, 2019, 1:32 pm

Read access violation

Post by Calert » April 24th, 2019, 1:42 pm

So for the past few days I've been creating this game and everything seemed to worked fine till i made a dumb mistake. I used "Find and replace" on an entire solution. From that time, when i alter my map.cpp file, the program goes crazy (draws some dumb shit on the screen), doesn't read bmp files (or at least doesn't draw them). Another thing is that the program has problems pointing to variables, eg. I set a variable to -3000 in map.h file and he reads it as 793, which the data stored in other variable. Also, there are those Miscellaneous files created outside of engine when debugging.
https://imgur.com/a/PoycuJT

Calert
Posts: 3
Joined: April 24th, 2019, 1:32 pm

Re: Read access violation

Post by Calert » April 24th, 2019, 2:55 pm


albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Read access violation

Post by albinopapa » April 24th, 2019, 3:38 pm

Sounds like you are going to need to do some debugging. Set some break points, step through the code, check the call stack and see where things go wrong.

The read access violation could probably be from trying to free memory that was allocated by operator new or trying to free memory that wasn't allocated at all or has already been deallocated.

You could try starting a new project and copy over all your old files into the new project, that has worked for me in the past if I know my stuff should work, but isn't.

If all else fails, zip it up and upload it or send a link to the repository so we can help debug it. It's definitely easier to debug and troubleshoot hands on.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Calert
Posts: 3
Joined: April 24th, 2019, 1:32 pm

Re: Read access violation

Post by Calert » April 24th, 2019, 4:20 pm

Alright so I repaired VS via VS installer and the problem with delete_scalar.cpp is supposedly gone. Now there is a problem "Can't access memory" concerning dword in Graphics.cpp or in Surface.cpp
delete[] pPixels;
pPixels = nullptr;
zip file: https://www.dropbox.com/s/2vlazkzp5iiyi ... k.zip?dl=0

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Read access violation

Post by albinopapa » April 25th, 2019, 4:23 am

Oh ma'gosh, having troubles finding the issue, but the error happens in the destructor of the Surface class. I've tried a few things and even switching to a std::unique_ptr instead of using new/delete and am still having the issue. It's possible somewhere you are writing past the end of a surface or some other class and corrupting the data.

I think I've got some other ideas to test that theory.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: Read access violation

Post by chili » April 25th, 2019, 6:07 am

Calert wrote:
April 24th, 2019, 1:42 pm
So for the past few days I've been creating this game and everything seemed to worked fine till i made a dumb mistake. I used "Find and replace" on an entire solution. From that time, when i alter my map.cpp file, the program goes crazy (draws some dumb shit on the screen), doesn't read bmp files (or at least doesn't draw them). Another thing is that the program has problems pointing to variables, eg. I set a variable to -3000 in map.h file and he reads it as 793, which the data stored in other variable. Also, there are those Miscellaneous files created outside of engine when debugging.
https://imgur.com/a/PoycuJT
You need to accept your LORD and Savior, Git, into your life my son.
Chili

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Read access violation

Post by albinopapa » April 25th, 2019, 6:47 am

Ok, so I found a clue. In your Map class, you have:
int LastEndpointX[100]
int LastEndpointY[100]

but you initialize around 125-126 of them, perhaps on oversight.
I would suggest using std::array instead of C-Style arrays. It will catch stuff like this for you. You could also consider using std::vector instead. This way you can just have the size of the vector maintained for you instead of keeping a "count" variable around.

That seems to have fixed the issue. I changed the array size to 300 like the FirstEndpointX and FirstEndpointY variables.

To use std::array, #include <array>.
Declare the array: int std::array<int, 300> FirstEndpointX;
use like C-Style arrays: FirstEndpointX[ 0 ] = 32;
Regular for loop: for (int i = 0; i < Lines; i++) FirstEndpointX[ i ] -= MapAdjustmentX;
Ranged for loop: for( const auto& troop : troops ) troop.Draw( wnd.mouse.GetX(), wnd.mouse.GetY() );

For std::vector, it's basically the same thing except the size isn't constant.
Include the header: #include <vector>
Declare a vector: std::vector<Troop> troops
Declare a vector with a preset size: std::vector<Troop> troops( 100 );
Add an element to a vector: troops.push_back( Troop() );
Add an element and get a reference to it: Troop& troop = troops.emplace_back(); // Only available if you have your settings setup for C++17 or latest.

There are a few ways to remove dead/unneeded elements from a vector, but I think the preferred method is:

Code: Select all

// Include the header file for the remove_if algorithm
#include <algorithm>

// Run the algorithm, which moves all the dead troops to the end of the container
auto iter = std::remove_if( troops.begin(), troops.end(), 
     [&]( const Troop& _troop ){ return !_troop.alive;} );

// Use the std::vector::erase() function to remove all elements from the first dead to the end of the container.  
troops.erase( iter, troops.end() );
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Read access violation

Post by albinopapa » April 25th, 2019, 7:11 am

Here are some peaces of advice.

Use references instead of pointers when passing objects to functions.
Use const references when possible.
Use the STL containers like std::array or std::vector instead of C-Style arrays.
Use const on variables when possible.
Organize your code so that Update code and Drawing code are separate.
Use const on functions that aren't changing the state of the object the function belongs to ( Draw functions and any Get function are a prime example )
Use enums instead of bools to represent different states like what state the game is in:

Code: Select all

enum class GameState{ Menu, Play, GameOver };
enum class TroopState{ Idle, Walking, Attacking, Dead };
I think you'll find it helps keep code a little more organized and easier to read through that way.
Use the classes like Troop to handle it's own update code moving it out of the Game class. The Game classes responsibilities are game flow, delegate the responsibilities that belong to the other classes to them.
If some action affects the flow of the game such as the player dying, then that would go in Game, if it only affects the objects, then it goes in a member function of that class.
You don't have to pass in variables belonging to a class to work on them. Your Troop::Draw function for instance can just use it's own instance of the variables instead of having to pass all of them. When you do need to pass troops variables, decide if it would be better/easier to just pass in the entire object by reference or const reference.

Code: Select all

// You have this:
troop[i].Draw(troop[i].x, troop[i].y, troop[i].type, troop[i].selected, troop[i].alive, troop[i].TroopSizeX, troop[i].TroopSizeY, gfx);

// When all you need is this:
troop[i].Draw( gfx );
Consider creating functions for code you find yourself writing over and over. For instance, checking if some point is inside a bounding rectangle seems to be rewritten a few times. You could create a Rect class that stores the left,top,right,bottom of the bounding area and create a member function that checks of a point is inside that area.
instead of :
if( x > left && x < right && y > top && y < bottom ) everywhere
you'd write:
if( GetRect().Contains( x, y ) ) // admittedly everywhere instead

Even make a Overlaps function that takes in another const Rect& object which tests if two bounding rectangles overlap.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Post Reply