Question about Polling [SOLVED]

[fovmester]

I have a problem with my game running out of memory after playing for some time. I am using quite a few repeatedly_execute_always functions and therefore I was wondering if there is any risk of AGS running out of memory when one is constantly polling in these functions. To examplify:

Code: ags

function repeatedly_execute_always() {
      if (mouse.y < 20) gGUI.Enable();
      else gGUI.Disable();
}

This code will poll the engine 40 times per second for the y-coord of the mouse and call either Disable or Enable 40 times per second? Is this a reasonable way to program or will it suck mem?

[Elliott Hird]

Meh, I've done stuff like that that does more.
It changed every mouse mode's cursor graphic and hotspot! 40 times a second! and there was no problems even on my crap machine.

[SSH]

That shouln't be a problem. Running out of memroy after a while indicates a memory leak somewhere. I would guess that you're allocating a lot of strings or dynamic sprites and they are n ot getting cleared up correctly: either down to your code or a bug in AGS

[Pumaman]

In what way is it running out of memory? Are you getting an error message? Can you supply more details?

[fovmester]

I started my game and let it run in a room with some animating objects.  I didn't move anything or do anything in the game. I just let it be.
After about half an hour I the game crashed and I got this error message:


---------------------------
Adventure Game Studio
---------------------------
An internal error has occured. Please note down the following information.
If the problem persists, contact Chris Jones.
(ACI version 2.71.886)

Error: Out of memory: failed to allocate 32204 bytes (at PP=334)

---------------------------


I've got 750MB RAM so it's not the computer that's the problem. Somewhere there must be some kind of memory leak. Either in the engine or caused by my code.

I looked into what I was doing every loop and found these things that could maybe be a problem:

Code: ags

Hotspot *hot = Hotspot.GetAtScreenXY(x, y);
Object *obj = Object.GetAtScreenXY(x,y);
Character *ch = Character.GetAtScreenXY(x, y);
InventoryItem *inv = InventoryItem.GetAtScreenXY(x,y);

I do this in repeatedly_execute to see what the mouse cursor is currently over. I then check which of these pointers are not null and change the status field accordingly.

NOTE: I just realized that this code would be much simpler if there was a common base class that Hotspot, Object, Character and InventoryItem were derived from.


In repeatedly_execute_always I do this:

Code: ags

string buffer;
 Overhp.GetText(buffer);
 int width = GetTextWidth(buffer, Overhp.Font);
 if (width>=0 && width<320) {
	Overhp.Width = width + 1;
	gLabel.Width = width + 1;
 }

which changes the width of the statusline (Overhp) to match the text in it.

I also have a module which has a repeatedly_execute_always function:

Code: ags

#define SPEED 4
#define CONSTANT 0
#define STD_SPEED 2

struct FaderGui {
  short fading; 
  int speed;
	// fading:
	// less than CONSTANT = fade out, 
	// equal to CONSTANT = don't fade
	// larger than CONSTANT = fade in
};

FaderGui guis[AGS_MAX_GUIS];


static function GUIFader::fadeInGUI(int guiNbr,int speed) {
		guis[guiNbr].fading = 1;		
		guis[guiNbr].speed = speed;
}

static function GUIFader::fadeOutGUI(int guiNbr,int speed) {
		guis[guiNbr].fading = -1;		
		guis[guiNbr].speed = speed;		
}



function repeatedly_execute_always() {
	int nbrOfGuis = GetGameParameter(GP_NUMGUIS, 0, 0, 0);
	int count = 0;
	while (count < nbrOfGuis) {
			GUI *theGUI = gui[count];
			int trans = theGUI.Transparency;
			
			if (guis[count].fading > CONSTANT) {
				//fade in
				
				trans-=guis[count].speed;
				if (trans<=0) { 
					trans = 0;
					guis[count].fading = CONSTANT;
				}
				theGUI.Transparency=trans;
				
			} else if (guis[count].fading < CONSTANT) {
				//fade out
				trans+=guis[count].speed;
				if (trans>=100) { 
					trans = 100;
					guis[count].fading = CONSTANT;
				}
				theGUI.Transparency=trans;
			} 
			
			count++;		
	}
}

This module can fade a gui in or out without blocking. I use this for fading in the Inventory Screen when the mouse cursor is in y = 0...20 pixels.


This is what I could find in my code that could have caused this error. I hope you find the problem without to much work, CJ!!

[SSH]

Firstly, if it is reproducable, why not comment out suspicious bits of code and see which one is doing it. You can probably tella bit sooner if it's going to die by using the windows task manager and checking the memory used by the app there. This will be easier in windowed mode, obviously.

Code: ags

string buffer;
 Overhp.GetText(buffer);
 int width = GetTextWidth(buffer, Overhp.Font);
 if (width>=0 && width<320) {
	Overhp.Width = width + 1;
	gLabel.Width = width + 1;
 }

which changes the width of the statusline (Overhp) to match the text in it.

My gut feeling would be that I would suspect this code most. I'm not sure why resizing a GUI to the text it contains would be necessary, unless the GUI provides a border or backgroudn to the text. Maybe there's a memory leak in the GUI resizing code in AGS: does the GUI have a BG image that gets clipped? Mayeb the dynamic sprite I guess that is created for the clipped background doesn't get cleared up  properly, or something? The declaring of the pointers shouldn't matter as they just create another reference to an existing object...

As for your module: interesting, but why not just combine speed and fading into one? then you could do:

Code: ags

#define SPEED 4
#define CONSTANT 0
#define STD_SPEED 2

struct FaderGui {
  int fading; 
	// fading:
	// larger than CONSTANT = fade out, 
	// equal to CONSTANT = don't fade
	// less than CONSTANT = fade in
};

FaderGui guis[AGS_MAX_GUIS];


static function GUIFader::fadeInGUI(int guiNbr,int speed) {
		guis[guiNbr].fading = -speed;		
}

static function GUIFader::fadeOutGUI(int guiNbr,int speed) {
		guis[guiNbr].fading= speed;		
}



function repeatedly_execute_always() {
	int nbrOfGuis = GetGameParameter(GP_NUMGUIS, 0, 0, 0);
	int count = 0;
	while (count < nbrOfGuis) {
			GUI *theGUI = gui[count];
			int trans = theGUI.Transparency;
			
			if (guis[count].fading != CONSTANT) {
				//fade in
				
				trans+=guis[count].fading;
				if (trans<=0 && guis[count].fading<0) { 
					trans = 0;
					guis[count].fading = CONSTANT;
				} else if (trans>=100 && guis[count].fading>0) {
					trans = 100;
					guis[count].fading = CONSTANT;
				}

				theGUI.Transparency=trans;
				
			} 
			count++;		
	}
}

[fovmester]

My gut feeling would be that I would suspect this code most. I'm not sure why resizing a GUI to the text it contains would be necessary, unless the GUI provides a border or backgroudn to the text. Maybe there's a memory leak in the GUI resizing code in AGS: does the GUI have a BG image that gets clipped? Mayeb the dynamic sprite I guess that is created for the clipped background doesn't get cleared up  properly, or something? The declaring of the pointers shouldn't matter as they just create another reference to an existing object...

The GUI has no borders and no background image so I don't think there's any Dynamic Sprite being created. I show the status bar next to the cursor so I need to control the size so that the text is always on one line.

I didn't show you that I also reposition the statusbar-GUI every loop so that it always stays next to the cursor. I use gLabel.SetPosition for this.

About the module. Nice change!!

[SSH]

The GUI has no borders and no background image so I don't think there's any Dynamic Sprite being created.

So much for taht idea!

I show the status bar next to the cursor so I need to control the size so that the text is always on one line.

Why not just leave it as 320 wide?

[fovmester]

Because I use the width to determine where it is on the screen, and stop it from getting outside the screen. I'm not sure but I think the game would crash if the GUI would be drawn outside the screen? Or is that just if its coordinates are outside the screen? Because if that is the case, then you are right: I don't need to change the width. I can just use the text width to make sure that THE TEXT never leaves the screen.

I will try to comment out things and see if it helps, but that will take some time so I won't be able to do it today.

[SSH]

It doesn't matter if the GUI is offscreen. I've done it for my "tooltip" GUI before.

Hopefully the mighty Puma might offer a solution before you have to comment things out, but who can fathom his mysterious ways? I was just offering a possible way to track things down if he doesn't answer soon for whatever reason...

[Pumaman]

Strange.

Can you open Task Manager, make sure the Memory Usage column is displayed (if not View, Select Columns to turn it on), and then leave your game running and see if the number seems to be gradually increasing?

Failing that, is your game small enough to upload for me to take a look at?

[Elliott Hird]

CJ, Games in production forum, woodlands demo

[Kweepa]

The memory usage growth is in direct proportion to the length of the status text - right click to get "Use" and it's slow (about 500k/sec) but with "Walk to stack of lumber" it grows at about 2500k/sec. I'd say that corresponds to leaking the graphic every frame (or four times per frame seemingly).

[fovmester]

I think SteveMcCrea has found our memory leak!! When I commented out the status line code (shown below) the memory usage was constant, but uncommenting it again made it increase gradually. I can't see WHERE in the code the problem is though. The code is taken from repeatedly_execute.

To understand the code you should know that I use some properties "Look at", "Use", "Pick up" and "Talk to" to get the text that will be shown for a certain command over a certain hotspot/object/character.

Ex: Talk to food -> Eat food
      Use tree -> Climb tree

And I differ between character/living objects and non living things when using the "Use inv"-command

Ex: Use herbs with Gorbye -> Give herbs to Gorbye

I hope someone can see what's the problem with this code!!!

Code: ags

		
                        string objectText;
			string commandText;
			
			int default = 1; //no inv,ch,hot or obj
			if (Mouse.Mode==eModeWalkto) {
				StrCopy(commandText, "Walk to");
				default = 0;
			}
				
			if (Mouse.Mode==eModeUseinv) {
			  string invName;
			  InventoryItem *in = cTom.ActiveInventory;
			  if (in!=null) {
					in.GetName(invName);
			  	StrCopy(commandText, "Use ");
					StrCat(commandText, invName);
					StrCat(commandText, " with");
					default = 0;
				} else StrCopy(commandText, "Use");			
			}
	
			int x = mouse.x;


			int y = mouse.y;
			Hotspot *hot = Hotspot.GetAtScreenXY(x, y);
			Object *obj = Object.GetAtScreenXY(x,y);
			Character *ch = Character.GetAtScreenXY(x, y);
			InventoryItem *inv = InventoryItem.GetAtScreenXY(x,y);
			
			
			if (inv != null) { //if clicking on inventory item don't show char,obj or hotspot text
			  obj = null; ch = null;
			  }
			if (obj!=null && ch!=null) {//making sure the object or the char in front gets handled
				if (obj.Y > ch.y) ch = null;
				else obj = null;
			}
			
			string buffer;
			if (inv != null) {
				default = 0;
				inv.GetName(objectText);				
				if (Mouse.Mode==eModeLookat) {				  
					inv.GetPropertyText("Look at", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeInteract) {				  
					inv.GetPropertyText("Use", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeTalkto) {				  
					inv.GetPropertyText("Talk to", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModePickup) {				  
					inv.GetPropertyText("Pick up", buffer);
					StrCopy(commandText, buffer);
				}				
			} else if (hot != hotspot[0]) {
			  default = 0;
				hot.GetName(objectText);				
				if (Mouse.Mode==eModeLookat) {				  
					hot.GetPropertyText("Look at", buffer); //gets the appropriate text for the look-command
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeInteract) {				  
					hot.GetPropertyText("Use", buffer);//gets the appropriate text for the use-command
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeTalkto) { //gets the appropriate text for the talk to-command
					hot.GetPropertyText("Talk to", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModePickup) {	//gets the appropriate text for the pickup-command
					hot.GetPropertyText("Pick up", buffer);
					StrCopy(commandText, buffer);
				}				
			} else if (obj != null) {
			  default = 0;
				obj.GetName(objectText);
				if (Mouse.Mode==eModeUseinv && obj.GetProperty("noWalkView")==1) { 
					//this is a hack to differ between living 
					//and not living objects: GetProperty("noWalkView")==1 for living things
					string invName;
					InventoryItem *in = cTom.ActiveInventory;
					if (in!=null) {
						in.GetName(invName);
						StrCopy(commandText, "Give ");
						StrCat(commandText, invName);
						StrCat(commandText, " to");
					default = 0;
					} else {
						ch.GetPropertyText("Use", buffer);
						StrCopy(commandText, buffer);
					}	
				} else if (Mouse.Mode==eModeLookat) {				  
					obj.GetPropertyText("Look at", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeInteract) {				  
					obj.GetPropertyText("Use", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeTalkto) {				  
					obj.GetPropertyText("Talk to", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModePickup) {				  
					obj.GetPropertyText("Pick up", buffer);
					StrCopy(commandText, buffer);
				}
			} else if (ch != null) {
			  default = 0;
			  StrCopy(objectText,ch.Name);
			  
				if (Mouse.Mode==eModeUseinv) {
					string invName;
					InventoryItem *in = cTom.ActiveInventory;
					if (in!=null) {
						in.GetName(invName);
						StrCopy(commandText, "Give ");
						StrCat(commandText, invName);
						StrCat(commandText, " to");
					default = 0;
					} else {
						ch.GetPropertyText("Use", buffer);
						StrCopy(commandText, buffer);
					}	
				} else if (Mouse.Mode==eModeLookat) {				  
					ch.GetPropertyText("Look at", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeInteract) {				  
					ch.GetPropertyText("Use", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModeTalkto) {				  
					ch.GetPropertyText("Talk to", buffer);
					StrCopy(commandText, buffer);
				} else if (Mouse.Mode==eModePickup) {				  
					ch.GetPropertyText("Pick up", buffer);
					StrCopy(commandText, buffer);
				}
			} else StrCopy(objectText,"");
			
			if (default==1) { //not over anything special
			  if (Mouse.Mode == eModeLookat) 
					StrCopy(commandText, "Look at");
				else if (Mouse.Mode == eModeTalkto) 
					StrCopy(commandText, "Talk to");
				else if (Mouse.Mode == eModePickup) 
					StrCopy(commandText, "Pick up");
			  else if (Mouse.Mode == eModeInteract) 
					StrCopy(commandText, "Use");
			}
			
			string text;
			StrCopy(text, commandText);
			StrCat(text, " ");
			StrCat(text, objectText);
			
			if (player.Room == 0) StrCopy(text, ""); //in titlescreen
			//display text
			gLabel.Controls[0].AsLabel.SetText(text);
			  
		

   

[Elliott Hird]

It's a far cry, but try and make all of the status updating code only happen if GetLocation etc isn't nothing.

[Gilbert]

Try moving all the "string xxx" declarations outside of the repeatedly_execute() function, currently most of them are dynamically declared and freed every gameloop, which can cause slow down or problems.

[Pumaman]

Thanks for bringing this up.

I've found the problem, there's a memory leak in the GUI.SetSize/Width/Height methods, when you change the GUI size. Since you do this in repeatedly_execute, it's mounting up pretty fast.

I'll get it fixed.

[Gilbert]

Hehe, but since from his codes he's using the "latest stable version" (i.e. V2.7), fixing it would mean he'll need to upgrade to the "not yet stable version" (i.e. V2.71), and he may need to update the string codes for his game to work... unless you compile a service release binary of the V2.7 engine.

[SSH]

Hah! I suspected it was the GUI resizing! Go SSH, Go SSH, Go SSH!

[fovmester]

Hehe, but since from his codes he's using the "latest stable version" (i.e. V2.7), fixing it would mean he'll need to upgrade to the "not yet stable version" (i.e. V2.71), and he may need to update the string codes for his game to work... unless you compile a service release binary of the V2.7 engine.

Well I was using 2.7 but now I use 2.71, and it compiled well. I think 2.71 is backward compatible!!

Great that you found it, CJ!