Can I make a pointer of an undefined type? [solved]

[Lord Vetinari]

This is a follow-up to my old thread about a custom interaction menu: https://www.adventuregamestudio.co.uk/forums/index.php?topic=54624.0

I'm still usign the code you see in post 3, with updated variables names to fix the silly quasi-hungarian notation. I'll just quote the post for simplicity's sake:

Code: ags

function on_mouse_click(MouseButton button) {
  // called when a mouse button is clicked. button is either LEFT or RIGHT
  Hotspot *activeHotspot;
  if (IsGamePaused() == 1) {
    // Game is paused, so do nothing (ie. don't allow mouse click)
  }
  else if (button == eMouseRight)  {
    //Here I put all the menu code.

    //Begin by resetting everything before a fresh start on a new hotspot
    gInteractionsMenu.Visible = false;  //This one is required because I had some fringe cases when a previous menu remained visible but with no options in the listbox, no idea why.
    ListInteractions.Clear();
    gInteractionsMenu.Height = 38;
    ListInteractions.Height = 0;
    
    //set the required control variables
    int AreThereInteractions = 0;
    vIntMouseActionX = mouse.x;
    vIntMouseActionY = mouse.y;
    activeHotspot = Hotspot.GetAtScreenXY(mouse.x, mouse.y);
    
    if(activeHotspot != null) //safety check: pointers can be "null", and if so you can't call methods on them (Thanks Snarky!)
    {
      //We add the options with a loop that automatically ends when there are no more options to be added
      vIntActionNumber = activeHotspot.GetProperty("Interaction_Number");
      String CurrentOption;
      int vActionCounter = 1;
      while (vIntActionNumber > 0) {
        CurrentOption = String.Format("Action_%d", vActionCounter);
        ListInteractions.AddItem (String.Format("%s", activeHotspot.GetTextProperty(String.Format("%s", CurrentOption))));  
        //The GUI is resized depending on how many items there are in the listbox
        gInteractionsMenu.Height += 23;
        ListInteractions.Height += 23;
        vIntActionNumber -=1;
        AreThereInteractions += 1;
        vActionCounter +=1;
      }
        if (AreThereInteractions > 0) {
        //If there are no options, the menu is not displayed.
        //I know that it would be enough to set this to 1 and call it a day, but I thought that adding and additional if clause in the loop to check it was superfluous
        gInteractionsMenu.X = mouse.x;
        gInteractionsMenu.Y = mouse.y;
        ListInteractions.SelectedIndex = -1;

        //Now we title the menu with the name of the hotspot
        if (activeHotspot.ID != 0) {
          lbMenuTitle.Text = String.Format("%s", activeHotspot.Name);
        }
        else {
          lbMenuTitle.Text = "Room";
          //This is here because sometimes even hotspot[0] has special interactions that are meant
          //to be available everywhere in the room, not only on actual interactive items.
          //For all the room this is not the case, the menu won't even appear if you don't click on an actual hotspot because AreThereInteractions is 0.
        }
        gInteractionsMenu.Visible = true;
      }
    }
  }
  else if (button == eMouseLeft) {
  cEgo.Walk(mouse.x, mouse.y eWalkableAreas);
  }
}

Now, that menu is supposed to work not only for hotspots, but for objects and characters as well.
Currently it does, but again I feel I did it in the least smart way possible: I set three pointers (Hotspot *activeHotspot, Object *activeObject, Character *activeCharacter), check each one at a time if it's not null (plus an additional safety check to exclude hotspot 0 when it comes to objects and characters), and copy/paste under each check the same code, just replacing the proper pointer each time.

I don't know much about coding, but I know that if you're copypasting the same lines of code several times, you're doing it the silly way and there's usually a better way of doing it. This has been bothering me for a while and today I feel in the right mood to try and fix it, but I can't seem to find a way. What should I look to understand what I should do? I thought enums, but, other than not knowing if I can make an enum with pointers, I still have to check which option of the enum is the right one, so that's not it.

[Cassiebsg]

Without checking your code through, I can suggest that you create a function with the repeated code, then instead of copy/pasting the code, just call the function...

As in:

Code: ags

function repeated_code() // use a name you'll remember what it is for later... ;)
{
    // Put your code here
}

function on_mouse_click(MouseButton button)
{
    // Make your checks
    repeated_code(); // calls your repeated lines
    // whatever comes after and new check
    repeated_code(); // calls your repeated lines
    // etc...
}

You can also add pointers to the new function, if you need to change a variable or two in the repeated code.

[Lord Vetinari]

I thought about it, but I feel it would still need a lot of unnecessary checks to replace the pointers inside the function.

[Snarky]

Refactor the code so that all the menu-building stuff is independent of how you grab the data from the hotspot/object/character in the first place, and you won't need to repeat it.

The function GetLocationType() may be useful to simplify the logic.

[RickJ]

You may want to consider learning how to write modules.  By the nature of your question it would youappear up to it and would be able to benefit with regard to eliminating redundant code. DRY.

[Lord Vetinari]

@RickJ: I wouldn't know where to start with that. I'm a self taught programmer, my confidence in my coding skill is rather low, and I can assure you that if anything I write seems somewhat advanced, it's pure luck 
I'm trying to expand my boundaries with this project, so I'll look into that, but I'd rather proceed with baby steps. 

@ Snarky That kinda helped, thank you.
Except that now instead of duplicating the code that creates the gui, I'm duplicating the code that grabs the data and I'm using a couple more variables to boot (specifically, an array of strings where I store all the interaction names to retrieve them later when I create the GUI and another INT counter to know how many elements of the array are actually used). Overall it's a couple of lines shorter than the previous version, still I don't think I've achieved that much. A couple more hints, please?

The issue with the way I'm approaching it is that I can't figure out where I should define the pointer, because if I nest that inside a couple of if clauses like

Code: AGS

 if (GetLocationType(mouse.x, mouse.y) == eLocationHotspot) {
      Hotspot *activeInteractionPoint;
 }
 else if (GetLocationType(mouse.x, mouse.y) == eLocationObject) {
      Object *activeInteractionPoint;
 }
 else if (GetLocationType(mouse.x, mouse.y) == eLocationCharacter) {
      Character *activeInteractionPoint;
 }

then the compiler won't accept activeInteractionPoint outside that series of clauses, same if I do that inside another function and then call it inside on_mouse_click. I guess I need to define the pointer outside the if/else clauses to be able to use by anything outside them, but if I don't use if/else clauses I can't test what kind of object I have under my cursor. It does work if I put inside each if clause the code that grabs the value, but that leads to duplicating it three times.
Therefore I guess my approach is completely wrong, but I can't think at anything different.

[Vincent]

It doesn't work if you do something like this ?

Code: ags

// main global script file

Hotspot *activeInteractionPoint;
Object *activeInteractionPoint;
Character *activeInteractionPoint;
int lt;

function repeatedly_execute() 
{
   lt = GetLocationType(mouse.x, mouse.y);
   
   if (lt == eLocationCharacter)
   {
     
   }
   
   else if (lt == eLocationHotspot)
   {
     
   }
   
   else if (lt == eLocationObject)
   {
     
   }
   
   else if (lt == eLocationNothing)
   {
     
   }
}

[Lord Vetinari]

It doesn't work if you do something like this ?

Code: ags

// main global script file

Hotspot *activeInteractionPoint;
Object *activeInteractionPoint;
Character *activeInteractionPoint;
int lt;

function repeatedly_execute() 
{
   lt = GetLocationType(mouse.x, mouse.y);
   
   if (lt == eLocationCharacter)
   {
     
   }
   
   else if (lt == eLocationHotspot)
   {
     
   }
   
   else if (lt == eLocationObject)
   {
     
   }
   
   else if (lt == eLocationNothing)
   {
     
   }
}

Maybe I'm misreading it, but it doesn't seem to help with the duplicated code issue.
It seems your code tests what kind of activeInteractionPoint I clicked on, then let me do different things with each type. I need the exact opposite, a code that does the same thing irregardless of the type of object the pointer points to.

If you need more context, it's a custom verb menu that grabs the list of available verbs/interactions from the object's properties (list that of course varies depending on the particular object; I managed to get that working in the old version of the code that is more or less what you see in the OP. That version though worked only for hotspots. I extended it to objects and characters by simply replicating the same code three times and changing the pointers, but that feels like a kludge. What I'm looking for is a way to achieve that without the duplication, if possible).

[Vincent]

I think I just misreading too, sorry.
I am not sure but probably it could work something like this ?

Code: ags

int lt;
 
function repeatedly_execute() 
{
   lt = GetLocationType(mouse.x, mouse.y);
   
   if (lt == eLocationNothing)
   {
     // do nothing
   }
   
   else
   {
     // mouse is over something (character, object or hotspot)
     // do stuff
   }
}

[Snarky]

You've got to read the old thread, Vincent. He wants to read a custom property from the thing he's hovering over, whether that's a hotspot, object or character.

And no, I don't think there's any way around a little code duplication to read that custom property.

What you could do is to define a function like this:

Code: ags

int GetAnyProperty(Hotspot* hs, Object* obj, Character* chr, String propertyName)
{
  if(hs != null) return hs.GetProperty(propertyName);
  if(obj != null) return obj.GetProperty(propertyName);
  if(chr != null) return chr.GetProperty(propertyName);
  return -1;
}

... and something similar for GetTextProperty(). First initialize all the variables to null and use GetLocationType() to set the right one to its correct value, and you can use this function instead of the type-specific GetProperty() to extract the relevant property value.

Or you could wrap up all that stuff inside the function and call it something like GetMouseOverProperty().

[Vincent]

Yes, I just realize that. My mistake. 

[Lord Vetinari]

Ok, thanks everybody.

I managed to streamline things a bit with Snarky's example. The important thing here was understanding if my approach was wrong or not; it's good to know that even if the code was rough, I was on the right track.