Null Pointer Referenced Error On Mouse Click [SOLVED...I THINK]

Started by Tenacious Stu, Tue 04/10/2011 16:28:21

Previous topic - Next topic

Tenacious Stu

Hi

It doesn't happen every time, but ocasionally I will get this error when selecting/using inventory items.

Null Pointer referenced - On Mouse Click


Code: ags
function on_mouse_click(MouseButton button)
{
  CursorMode mode = eModeInteract;
  if ((button == eMouseRight) || (button == eMouseRightInv)) mode = eModeLookat;
  else if (player.ActiveInventory != null) mode = eModeUseinv;
  if ((button == eMouseLeftInv) || (button == eMouseRightInv))
  {
    InventoryItem *iat = InventoryItem.GetAtScreenXY(mouse.x, mouse.y);
    if (iat.IsInteractionAvailable(mode)) iat.RunInteraction(mode);
    else if ((button == eMouseLeftInv) && (player.ActiveInventory == null)) player.ActiveInventory = iat;
    else if ((button == eMouseRightInv) && (player.ActiveInventory != null)) player.ActiveInventory = null;
  }
  else if (IsInteractionAvailable(mouse.x, mouse.y, mode)) ProcessClick(mouse.x, mouse.y, mode);
  else if ((button == eMouseLeft) && (player.ActiveInventory == null))
  {
    ProcessClick(mouse.x, mouse.y, eModeWalkto);
  }
  else if ((button == eMouseRight) && (player.ActiveInventory != null))
  {
    player.ActiveInventory = null;
  }
}


It says the line number the error is happening is 363 which is this one:

Code: ags
if (iat.IsInteractionAvailable(mode)) iat.RunInteraction(mode);


The code was written by someone else, so unfortunatley I don't fully understand it. But other than this happening on rare ocassions it works fine, but I would rather nip this in the bud.

Thanks in advance for any help :)

pcj

#1
You need to make sure iat exists before performing operations on it.  If there is no inventory item under the cursor there will be an error.

Code: ags
function on_mouse_click(MouseButton button)
{
  CursorMode mode = eModeInteract;
  if ((button == eMouseRight) || (button == eMouseRightInv)) mode = eModeLookat;
  else if (player.ActiveInventory != null) mode = eModeUseinv;
  if ((button == eMouseLeftInv) || (button == eMouseRightInv))
  {
    InventoryItem *iat = InventoryItem.GetAtScreenXY(mouse.x, mouse.y);
    if (iat) {
      if (iat.IsInteractionAvailable(mode)) iat.RunInteraction(mode);
      else if ((button == eMouseLeftInv) && (player.ActiveInventory == null)) player.ActiveInventory = iat;
      else if ((button == eMouseRightInv) && (player.ActiveInventory != null)) player.ActiveInventory = null;
    }
  }
  else if (IsInteractionAvailable(mouse.x, mouse.y, mode)) ProcessClick(mouse.x, mouse.y, mode);
  else if ((button == eMouseLeft) && (player.ActiveInventory == null))
  {
    ProcessClick(mouse.x, mouse.y, eModeWalkto);
  }
  else if ((button == eMouseRight) && (player.ActiveInventory != null))
  {
    player.ActiveInventory = null;
  }
}


(The code could be simplified somewhat due to some other logic mistakes but it shouldn't throw an exception)
Space Quest: Vohaul Strikes Back is now available to download!

monkey0506

#2
If button is eMouseLeftInv or eMouseRightInv then InventoryItem.GetAtScreenXY(mouse.x, mouse.y) should (in theory) always return a non-null value.

This code almost looks like that "somebody else" could have been me, but I tend to be a stickler for checking pointers even when I know the null case should never be an issue ever (for example, I'm pretty sure Hotspot.GetAtScreenXY will never return null, but I still check them anyway).

Oh, and although I tend to forget about those pesky game.* variables, it would actually be safer in this case to use game.inv_activated:

Code: ags
InventoryItem *iat = inventory[game.inv_activated];


@pcj: What logic mistakes? I don't actually see any redundancy or fallacy in the logic in the original code snippet, with the possible exception of double-checking the enum value in favor of preventing duplication of pointer-creation code or unnecessarily checking for inventory items on-screen.

And while it compiles and runs fine, your usage of "(iat)" instead of "(iat != null)" is actually non-conventional for pointers (at least in AGS).

Tenacious Stu

I tried the code and it seems to be working, but like I say, this only happens on rare occasions. Like weeks and weeks in between when testing. But hopefully, it's all good now  ;D

Quote from: monkey_05_06 on Wed 05/10/2011 03:47:20
This code almost looks like that "somebody else" could have been me,

I believe it was you Monkey and thank you for helping me out  ;)

Thanks guys!

monkey0506

Well the absolute safest way to error-proof the code would be to do something along the lines of this:

Code: ags
function on_mouse_click(MouseButton button)
{
  CursorMode mode = eModeInteract;
  if ((button == eMouseRight) || (button == eMouseRightInv)) mode = eModeLookat;
  else if (player.ActiveInventory != null) mode = eModeUseinv;
  if (((button == eMouseLeftInv) || (button == eMouseRightInv)) && (game.inv_activated >= 1) && (game.inv_activated <= Game.InventoryItemCount))
  {
    InventoryItem *item = inventory[game.inv_activated];
    if (item.IsInteractionAvailable(mode)) item.RunInteraction(mode);
    else if ((button == eMouseLeftInv) && (player.ActiveInventory == null)) player.ActiveInventory = item;
    else if ((button == eMouseRightInv) && (player.ActiveInventory != null)) player.ActiveInventory = null;
  }
  else if (IsInteractionAvailable(mouse.x, mouse.y, mode)) ProcessClick(mouse.x, mouse.y, mode);
  else if ((button == eMouseLeft) && (player.ActiveInventory == null))
  {
    ProcessClick(mouse.x, mouse.y, eModeWalkto);
  }
  else if ((button == eMouseRight) && (player.ActiveInventory != null))
  {
    player.ActiveInventory = null;
  }
}

cat

I know this thread is quite old but as I didn't find a solution in the forums I thought I might post some info for the next person needing help with this issue:

I put my inventory in the iconbar and InventoryItem.GetAtScreenXY(mouse.x, mouse.y) returned null when clicking on an item. When I changed the iconbar visibility to "Normal, initially on" it worked, but not in "When mouse moves to top of screen" mode. My workaround was, as suggested by Snarky in the other thread to set it to Normal, initially on and use the Tween Module to show the GUI (which also adds a nice effect).

To sum up, I think inventory clicks are not handled correctly, if the GUI is in "When mouse moves to top of screen" mode.

SMF spam blocked by CleanTalk