Any way I can ask for a follow up? I'm hitting a weird memory breakpoint on vector erase, it doesn't give me a call stack for some reason. I've only seen that happen before if there is some corruption going on, which I hope this isn't the case. This is the context:

POINT corner = FindNearestSnapPoint({ int(xPos + 0.5f), int(yPos + 0.5f) });
				//make sure that there is no other bitmap in that cell, if so delete it
				auto vIter = LandSpriteList.begin();
				for (vIter; vIter < LandSpriteList.end(); vIter++)
				{
					if (corner.x == vIter->GetPosition().x && corner.y == vIter->GetPosition().y)
					{
						LandSpriteList.erase(vIter);
						break;
					}
				}

Just seeing if there is an image in the place where mouse is clicked already and erasing it from the list if so.

But what happens is that when the vector tries to move all the elements above the one deleted down, it breaks inside memcpy.

I'm not looking for an answer as to why as that probably involves looking over a bunch of other code, but it would help to know what is the exact sequence of events once erase is called?

LandSpriteList contains AnimatedBitmap, I set a break point in AnimatedBitmap destructor and for some reason it's not even called when the error happens, it just goes straight to memcpy, which is weird to me since I'd expect memcpy be called AFTER the element was already deleted or is something else happening here?

Also I'm not sure if this is part of normal behaviour, but since I set the breakpoint inside the destructor, I've noticed it. If I insert an element as such

AnimatedBitmap ins = *currentInsertElement;
				ins.SetPosition( corner.x, corner.y);
				LandSpriteList.push_back(ins);

It calls the destructor at the end, so far so good, we're destroying the "ins" temporary variable. Now I insert a second bitmap, again I hit the destructor at this point, but now twice, once for the first image once for the new one. And this happens as many times as there are bitmaps, so on the 4th insertion, the destructor is called 4 times. Is that something up with my code or am normal behaviour?

This is my destructor if it helps any:

AnimatedBitmap::~AnimatedBitmap()
{
	if (allLoaded.size())
	{
		vector<AnimatedBitmap*>::iterator thisIter;
		for (thisIter = allLoaded.begin(); thisIter < allLoaded.end(); thisIter++)
		{
			if ((*thisIter)->GetID() == this->mID)
			{
				thisIter = allLoaded.erase(thisIter);
				break;
			}
		
				
		}
	}
	vector<BITMAP_FILE>::iterator vIter;
	for (vIter = frames.begin(); vIter < frames.end(); vIter++)
	{
		Unload_Bitmap_File(&(*vIter));
	}
}

Or just not delete during iteration. You can push indices or iterator values in another vector, then just reverse iterate that and erase or otherwise remove from the primary vector as you go.

You could store iterators to erase separately, but depending on how your memory allocation is set up and how big your vectors are, the performance cost of a heap allocation for the second vector might be more than just erasing as you go. Once you know the proper way to do it, erasing as you go requires less code, less memory, and sometimes less time.

Or, again, you could just use the standard algorithms (which are probably less costly than doing it yourself - std::remove and std::remove_if move the elements around in a "smarter" way than straight iteration + erase) and only micro-optimize if your profiler tells you that removing things from vectors is a problem.

I'm far more concerned about gibberish code and swarms of bugs than performance.

std::remove_if is the best option if you don't mind that it's iterating the whole thing within itself. I assumed (perhaps incorrectly) that OP was talking about deleting things while doing other work on the vector elements. If you're already doing a pass then you may as well mark and sweep. I'm not sure why you're concerned about 'more code' here a vector push instead of deletion gymnastics will make your pass far easier to read. If performance really is an issue then just set the reserve to something reasonable on the auxiliary vector. I can't imagine it being a significant issue unless you're in a constrained environment.

tl;dq

First code segment:

#include <algorithm>

auto rmv_iter = std::remove_if(LandSpriteList.begin(), LandSpriteList.end(), [corner&](whatever_is_in_this_vector& thing) { return corner.x == thing.GetPosition().x && corner.y == thing.GetPosition().y; });
std::erase(rmv_iter, LandSpriteList.end());

Second code segment:

LandSpriteList.emplace_back(*currentInsertElement);
LandSpriteList.back().SetPosition(corner.x, corner.y);

Third code segment:

#include <algorithm>

AnimatedBitmap::~AnimatedBitmap() {
  std::erase(std::remove(allLoaded.begin(), allLoaded.end(), this), allLoaded.end());

  //give the frames RAII and shove them in a vector and you won't have to fret about this
  for(auto& frame : frames) { Unload_Bitmap_File(&frame); }
}

I have no idea where your problems are coming from because I can't see the larger context of your code. If you can get on the chat you can probably get better help with this in real time.

I'm talking about the performance hit incurred by the heap allocation itself. It's probably fine if you're only doing it once a frame, but if you're doing this a lot of times a frame, that's probably bad.

I was thinking more along the lines of doing it once, maybe twice, per thread.

--------

VanillaSnake came to the chat and we resolved OP. The issue was that AnimatedBitmap created some external references to itself (in 'allLoaded'). LandSpriteList is a vector of AnimatedBitmap and when it would reallocate or shuffle the vector the external refs would become invalidated, but were later being dereferenced, causing access errors.

If we are talking about vector container and multiple erasings on single iteration...

Best way do not erase on itaration, becouse it will be data copy nightmare.

You can instead of erasing element copy to its place next element and on end iteration resize vector;

This will guaraty that one element wount be copied more than one time + resize copy.