oh, look, a memory leak!

[ReiquelApplegate]

In weapons.cpp:

w=this; //Oh, look, a memory leak. The new instruction is making something on the heap, but this circumvents removing it.

[ReiquelApplegate]

ewMagic has this problem many times

[wilsonpe66]

There is a lot of copy pasta for that line :)

[wilsonpe66]

I think all of the following logic cout be put into is own fuction to avoid copy pasta here.

weapon *w=NULL;
					
					if(id==ewMagic)
					{
						w=new weapon(*this);
						Lwpns.add(w);
						dead=0;
					}
					else
					{
						w=this; //Oh, look, a memory leak. The new instruction is making something on the heap, but this circumvents removing it. 
					}
					
					w->o_tile = ref_o_tile;
					w->tile = ref_o_tile;
					w->dir = 3-w->dir;
					w->doAutoRotate(true);
					
					if(w->id != wWind)
					{
						w->id = wRefMagic; w->convertType(true);
						if ( do_animation ) 
						{
							if((w->dir==1)||(w->dir==2))
							w->flip ^= 3;
						}
					}
					if ( do_animation ) 
					{
						w->tile=w->o_tile;
						
						if(w->dir&2)
						{
							if(w->frames>1)
							{
							w->tile+=w->frames;
							}
							else
							{
							++w->tile;
							}
						}
					}
					w->ignoreHero=false;
					w->ignorecombo=((int32_t(checky)&0xF0)+(int32_t(checkx)>>4));
					w->y=(int32_t(checky)&0xF0)+check_y_ofs;
					w->x=(int32_t(checkx)&0xF0)+check_x_ofs;

[wilsonpe66]

see the following warning

[wilsonpe66]

This is a problem

if(id==ewMagic)
					{
						w=new weapon(*this);
						Lwpns.add(w);
						dead=0;
					}

the application is heap allocating here but it is not be clean up in w pointer goes out of scope.

				{
					weapon *w=this;
					bool isLocalHeapAllocated = false;

					if(id==ewMagic)
					{
						w=new weapon(*this);
						Lwpns.add(w);
						dead=0;
						isLocalHeapAllocated = true;
					}

					w->o_tile = ref_o_tile;
					w->tile = ref_o_tile;
					w->dir = 3-w->dir;
					w->doAutoRotate(true);
					
					if(w->id != wWind)
					{
						w->id = wRefMagic; w->convertType(true);
						if ( do_animation ) 
						{
							if((w->dir==1)||(w->dir==2))
							w->flip ^= 3;
						}
					}
					if ( do_animation ) 
					{
						w->tile=w->o_tile;
						
						if(w->dir&2)
						{
							if(w->frames>1)
							{
							w->tile+=w->frames;
							}
							else
							{
							++w->tile;
							}
						}
					}
					w->ignoreHero=false;
					w->ignorecombo=((int32_t(checky)&0xF0)+(int32_t(checkx)>>4));
					w->y=(int32_t(checky)&0xF0)+check_y_ofs;
					w->x=(int32_t(checkx)&0xF0)+check_x_ofs;

					if (isLocalHeapAllocated)
					{
						delete w;
					}
				}

[ZelRai]

This is why we don't like Conner's auto pointers and new->this intailizers. They are great ideas but a measure of direction and sense is needed.

…

On Thu, Sep 5, 2024, 10:04 PM Peter Wilson ***@***.***> wrote: see the following warning image.png (view on web) <https://github.com/user-attachments/assets/a64edf89-182d-45f0-93a8-148ac3fd8035> — Reply to this email directly, view it on GitHub <#27 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV3W6LTRHHKQP5DR43GZLDZVEER7AVCNFSM6AAAAABGMXJO56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZTGA2DKMJSGE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[wilsonpe66]

well there are 8 location to change. And I have analyzed a lot of the code base and found object life times and encapsulation to be very weak.

In the past I have tryied clean it up on my one forks but there still so many more issues.

[wilsonpe66]

Also it is dangerous to use memset and memcpy on non trival stucts and classes.

See https://en.cppreference.com/w/cpp/language/classes#Trivial_class

[wilsonpe66]

I will creeate a pr for this issue.

[wilsonpe66]

There is an issue I have to try to resolve after all

Lwpns.add(w);

[wilsonpe66]

I hate globals :( I am back to the drawing board for now.

[wilsonpe66]

Did anyone try making the sprite_list counter and std::atomic and see if that can help?

I am going to try an attack the problem from that angle

[ZelRai]

Actually that may be a good idea. It doesn't cost much to make something atomic and if it saves a thread crash, resources well spent.

…

On Thu, 5 Sept 2024 at 23:03, Peter Wilson ***@***.***> wrote: Did anyone try making the sprite_list counter and std::atomic and see if that can help? I am going to try an attack the problem from that angle — Reply to this email directly, view it on GitHub <#27 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV3W6OW5UBGSW6Y35CNHNDZVELPVAVCNFSM6AAAAABGMXJO56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZTGEZTCNZYGE> . You are receiving this because you commented.Message ID: ***@***.***>

[wilsonpe66]

I in my most local change and getting 100 frame per second boost at least.

[wilsonpe66]

If remove the frame cap, get like 2000 frames per second.

[wilsonpe66]

If I remove the frame cap, in release build plays get like 4000 to 6000 frames per second. Before I would get like 450 frame per second.

[wilsonpe66]

I have created my pr #29

[ZelRai]

Nice work.