Is there a way to read a hotspot's poperty without knowing the hs ID? [Solved]

[Lord Vetinari]

A bit of context: I made a game last year, a quite crappy one if I say so myself, by playing it VERY safe, using the most basic scripting and interactions and the least amount of custom made stuff based on examples, code monkey style. Now I'm trying to spice things up and customize things and generally trying to be more daring by upgrading/remaking it with some neater features (like the clock I asked about last time).

What I'm trying to replicate at the moment is something like Leisure Suit Larry 7's interactions menu. If you are not familiar with it, LSL7 used left click to move the character and possibly to execute the default interaction with something (but I'm not sure; I haven't replayed it recently and anyway I'm not really interested in this second functionality), and right clicks opened a small drop-down menu at the cursor's position with a list of potential interactions with that object/hotspot that changed depending on what you clicked on. It was also used to access the inventory and the ship's map and possibly some other generic options I can't remember when you right-clicked anywhere outside an interactive element (but again I'm not interested in that as much - which is kinda ironic, because it seems it's the only bit I can replicate as I know that no hotspot in AGS script is hotspot[0]).
I like this kind of interface, it's somewhat of a mix between the complexity of having multiple modes/verbs and the simplicity of the more modern one-click-does-everything systems. It also allowed to sneak in the occasional nice joke interactions (well, actually coarse joke interactions since we're speaking of LSL, but that's beyond the point).
I know, I'm trying this despite tha fact that a couple of days ago I wasn't able to properly format a string containing a couple of variables (later I found that my mistake is explained in the manual too - really sorry for that -, but for some reason I find it hard to navigate the manual and all those legacy references to very old versions of AGS make it hard for me to understand how you are supposed to properly spell things now in 3.4); It does seem a bit excessive for my skills, but as Ron Gilbert said, "sometimes it pays to be so stupid that you don't understand you can't do what you're trying to do" (ok, I'll probably never be as good a coder as Ron Gilbert and I'm definitely not making something as groundbreaking as Maniac Mansion, but hopefully even I can manage to achieve something).

I kinda managed to have something that resembles what I described using a custom GUI with a listbox and resizing both depending on the amount of options available, and basically bruteforcing it by replicating the same code (or at least variations of the same given the different actions allowed on each object/hotspot) in every single interactive element in one room, which I know is very bad programming and a good hint you need a generic function.
So I created a bunch of custom text properties for objects, characters and hotspots called Action1, Action2, etc, and tried to create a generic function in the general script that reads those and use them to fill the listbox.
The problem is that I can't find a way to read them if I don't specify the ID of the hotspot. I tried to use

Code: ags

Hotspot.GetProperty(Action1)

but it doesn't work, the compiler returns an error and, since every example I can find of it's use on the manual and the wiki use

Code: ags

hotspot[somenumber].GetProperty(action1)

I figured that you need to know what specific hotspot you are clicking on.
I may work around that by starting with a series of checks like

Code: ags

if (Hotspot.GetAtScreenXY(mouse.x, mouse.y) == hotspot[1])

which I know it works because it comes straight from the manual (and I used it to check that hotspot[0] case I talked about), but that seems blunt and bruteforceish too.
Is there a way to get the values of those properties without knowing the hotspot ID in advance? Or am I approaching the matter from a completely wrong angle?
If I'm doing everything wrong I'd like you to just point me in the right direction rather than giving me actual working code, I'd like to understand and figure it out myself, otherwise I'll never stop opening threads on the help forums

[Snarky]

Puhh! Quite the essay there! As a tip, I think most of us prefer the questions to be a little more to the point: context is good if it helps understand the problem, but a lot of this isn't really relevant to the issue you're having.

Aaaanyway, the thing you're missing is that you can create a "hotspot pointer", which is a variable that you can set to refer to whatever hotspot you like. Pointers are indicated by a * when you declare them. You can then use the pointer variable just like an entry from the hotspot[] array. So, for example:

Code: ags

  Hotspot* activeHotspot = Hotspot.GetAtScreenXY(mouse.x, mouse.y);
  if(activeHotspot != null) // pointers can be "null", and if so you can't call methods on them
    activeHotspot.GetProperty(Action1);

You can also declare a function to take a hotspot pointer as an argument:

Code: ags

int getActionCount(Hotspot* hs)
{
  if(hs == null)
    return 0;
  if(hs.getProperty("action1") == 0)
    return 0;
  if(hs.getProperty("action2") == 0)
    return 1;
  if(hs.getProperty("action3") == 0)
    return 2;
  if(hs.getProperty("action4") == 0)
    return 3;
  
  return 4;
}

Which you can then call like so:

Code: ags

  Hotspot* activeHotspot = Hotspot.GetAtScreenXY(mouse.x, mouse.y);
  int actionCount = getActionCount(activeHotspot);
  // TODO: Display a list with that many entries

That's just an example, you probably wouldn't code it quite like this.

Edit: I changed the examples a bit to try to make them more relevant to your task.

[Lord Vetinari]

Thank you! I can be a rambly guy, I'll try to keep it at minimum, sorry. 

Bonus question: the brute force way I tried had all the code in the various AnyClick() functions; the current plan is to keep it that way and start replacing every bit that can be generalized with a global function. When I've done that, is there a better place to put the interactions code or it's fine where it is?

[ramble, avoid if not interested]
Can I say that AGS's manual and wiki lack a bit of cross referencing and sometimes assume that the users are a bit more knoweledgable that what's probably the average level?
I mean, unless you read and memorize the whole thing from top to bottom newbies like me will miss obvious stuff like this one. I searched before asking here (probably with the wrong wording), and everything pointed to the hotspost function and properties page. However there isn't on that page a line as simple as "all these functions require IDs, if you want to generalize them you need to use pointers", which would at least give me a clue on what to look after that.

[Lord Vetinari]

Nevermind, I think I solved everything myself and I'm really, really happy right now! I removed all the default code in the on_mouse_click function and added this:

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);
  }
}

Then, in the function that handles listbox selections:

Code: ags

function ListInteractions_OnSelectionCh(GUIControl *control)
{
  if (ListInteractions.SelectedIndex == 0) {
  vBoolAction1 = true;  //This variable is used in the hotspot code to choose what action needs to be performed.
  gInteractionsMenu.Visible = false; //Turn off the GUI
  Room.ProcessClick (vIntMouseActionX, vIntMouseActionY, eModeUsermode1); //process the interaction
  //reset the "fake mouse pointer" to a place where there is the unclickable statusbar, so it won't find anything to interact with for safety reasons.
  vIntMouseActionX = 0;
  vIntMouseActionY = 0;
  mouse.Mode = 1; //set the mouse to one of the mouse mode I'm not going to use
  }
  else if (ListInteractions.SelectedIndex == 1) {
  vBoolAction2 = true;
  gInteractionsMenu.Visible = false;
  Room.ProcessClick (vIntMouseActionX, vIntMouseActionY, eModeUsermode1);
  vIntMouseActionX = 0;
  vIntMouseActionY = 0;
  mouse.Mode = 1;
  }
  // Repeat the same bits for the maximum listbox choices possible (currently 8 because I never have more options than that, so up to ListInteractions.SelectedIndex == 7 and vBoolAction8
  else
  mouse.Mode = 1; //set the mouse to a mode that is not walking, so that it doesn't do anything as there is no code associated with any of the default actions
}

And finally, in each hotspot/room script (this is a single example, for a TV in a living room):

Code: ags

function hTV_Mode8()
{
  if (vBoolAction1 == true) {
    //do stuff
    vBoolAction1 = false;
  }
  if (vBoolAction2 == true) {
    //do stuff
    vBoolAction2 = false;
  }
}

//etc.

As you can see I bypassed all predefined mouse modes and interactions and used the first of the two custom ones because, even if I try to be constant regarding at which place certain actions should appear, it turns out that, since some interactions are not available on certain hotspots, there are so many exceptions that it's easier to manually put in the hotspot code the interactions you need.
I'm really happy, it works like a charm. It slightly bothers me that I have to use a "close" button to close it if you con't choose any option, maybe I should add a function in repetedly_execute that checks if the mouse is moved away from the GUI and deactivate it automatically.

What do you think of it? See any part that can be improved? I tested it in all the conditions I could think of and it seemed very reliable (at least in my game).

[Snarky]

What do you think of it? See any part that can be improved? I tested it in all the conditions I could think of and it seemed very reliable (at least in my game).

Good job!

So every hotspot can have up to 8 different interactions, but not always the same eight interactions? If they're always using a subset of the same eight, I think there are simpler/better ways to do it.

The main thing I would change if this were my code is the series of vBoolActionN flags. Assuming only one of them is true at a time, you shouldn't have 8 different variables to represent one thing. This a typical case for using an enum. An enum is a variable that can have a set of predefined values:

Code: ags

enum PlayerAction
{
  eActionInvalid = -1,
  eAction1,  // If you have a constant set of 8 interactions, I would use a descriptive action name instead of a number
  eAction2,
  eAction3,
  eAction4,
  eAction5,
  eAction6,
  eAction7,
  eAction8
};

And then to set the current action (I've also fixed your indenting):

Code: ags

PlayerAction currentAction;

function ListInteractions_OnSelectionCh(GUIControl *control)
{
  if (ListInteractions.SelectedIndex == 0) {
    currentAction = eAction1;  // <--- NOTE: Changed!
    gInteractionsMenu.Visible = false; //Turn off the GUI
    Room.ProcessClick (vIntMouseActionX, vIntMouseActionY, eModeUsermode1); //process the interaction
    //reset the "fake mouse pointer" to a place where there is the unclickable statusbar, so it won't find anything to interact with for safety reasons.
    vIntMouseActionX = 0;
    vIntMouseActionY = 0;
    mouse.Mode = 1; //set the mouse to one of the mouse mode I'm not going to use
  }
  // ...
}

And finally:

Code: ags

function hTV_Mode8()
{
  if (currentAction == eAction1) {
    //do stuff
  }
  else if (currentAction == eAction2) {
    //do stuff
  }
  // (other alternatives: you don't need entries for the actions not supported by this hotspot
  else {
    //default/fallback: in case you missed something
  }
  currentAction = eActionInvalid;
}

In fact, in AGS 3.4 you can now use the switch-case construct instead of the series of if-else-if statements:

Code: ags

function hTV_Mode8()
{
  switch(currentAction) {
    case Action1:
      //do stuff
      break;
    case Action2:
      //do stuff
      break;
    // ... more cases...
    default:
      //do stuff
      break;
  }
  currentAction = eActionInvalid;
}

The drawback here is that you can't tell what action the user has chosen without looking up the custom property. Unless the set of interactions is entirely open, I would prefer meaningful names and some other way to define which ones are supported by each hotspot.

Can I say that AGS's manual and wiki lack a bit of cross referencing and sometimes assume that the users are a bit more knoweledgable that what's probably the average level?
I mean, unless you read and memorize the whole thing from top to bottom newbies like me will miss obvious stuff like this one. I searched before asking here (probably with the wrong wording), and everything pointed to the hotspost function and properties page. However there isn't on that page a line as simple as "all these functions require IDs, if you want to generalize them you need to use pointers", which would at least give me a clue on what to look after that.

It's not a bad idea to read the manual beginning-to-end (skimming over the scripting documentation, just getting a sense of the classes that exist): it's not that long. But there are definitely aspects of it that could be improved, and getting feedback from beginners is important input for that work. I'm hoping I'll have a chance to take a pass at an update soonish (meaning sometime this year).

[Snarky]

Oh, one more concern: I'm curious about your variable naming scheme...

Code: ags

vBoolAction2
vIntMouseActionX

You're using some weird version of Hungarian notation, with two prefixes: v- (not sure what this means) and the variable type (Int- or Bool-). In my opinion this is not good practice. Most coding style guides recommend against Hungarian notation (particularly system-Hungarian, where it's used to indicate the variable type). It's used in AGS for a few things (mostly built-in struct types, and enum options), but I definitely wouldn't use it for ints and bools.

[Lord Vetinari]

So every hotspot can have up to 8 different interactions, but not always the same eight interactions? If they're always using a subset of the same eight, I think there are simpler/better ways to do it.

No, they change. In the current state of the project, the amount ranges from 0 (almost every hotspot[0] except two specific cases) to at maximum 8, with an average of 4/5.
They change with each hotspot for a couple of reasons:
- since you can't talk to a fridge, for example, I think there's no sense in adding a "talk" option, so some interactions may not be available. This is what makes it hard to figure out what is in a specific spot on the listbox. It is slightly confusing to have a series of Action_n properties, but since I code the interactions as I set up the hotspot and therefore the local values of each custom property, hopefully it's easy to remember which one is which.
- some of them indeed have custom names. Rather than a generic "use", it may be "turn on" for an electric appliance, "eat a snack" for the fridge, etc. In the case of the TV I put in the example post, it has two separate options for "watch movie" and "watch TV show", for example.

The main thing I would change if this were my code is the series of vBoolActionN flags. Assuming only one of them is true at a time, you shouldn't have 8 different variables to represent one thing. This a typical case for using an enum. An enum is a variable that can have a set of predefined values:

Code: ags

enum PlayerAction
{
  eActionInvalid = -1,
  eAction1,  // If you have a constant set of 8 interactions, I would use a descriptive action name instead of a number
  eAction2,
  eAction3,
  eAction4,
  eAction5,
  eAction6,
  eAction7,
  eAction8
};

And then to set the current action (I've also fixed your indenting):

Code: ags

PlayerAction currentAction;

function ListInteractions_OnSelectionCh(GUIControl *control)
{
  if (ListInteractions.SelectedIndex == 0) {
    currentAction = eAction1;  // <--- NOTE: Changed!
    gInteractionsMenu.Visible = false; //Turn off the GUI
    Room.ProcessClick (vIntMouseActionX, vIntMouseActionY, eModeUsermode1); //process the interaction
    //reset the "fake mouse pointer" to a place where there is the unclickable statusbar, so it won't find anything to interact with for safety reasons.
    vIntMouseActionX = 0;
    vIntMouseActionY = 0;
    mouse.Mode = 1; //set the mouse to one of the mouse mode I'm not going to use
  }
  // ...
}

And finally:

Code: ags

function hTV_Mode8()
{
  if (currentAction == eAction1) {
    //do stuff
  }
  else if (currentAction == eAction2) {
    //do stuff
  }
  // (other alternatives: you don't need entries for the actions not supported by this hotspot
  else {
    //default/fallback: in case you missed something
  }
  currentAction = eActionInvalid;
}

I'll look into enums, thanks. But can I set them as global variables? Can I set it up in the global scrip and then read them in the room hotspot function?

In fact, in AGS 3.4 you can now use the switch-case construct instead of the series of if-else-if statements:

Code: ags

function hTV_Mode8()
{
  switch(currentAction) {
    case Action1:
      //do stuff
      break;
    case Action2:
      //do stuff
      break;
    // ... more cases...
    default:
      //do stuff
      break;
  }
  currentAction = eActionInvalid;
}

The drawback here is that you can't tell what action the user has chosen without looking up the custom property. Unless the set of interactions is entirely open, I would prefer meaningful names and some other way to define which ones are supported by each hotspot.

This is very interesting! Indeed the interaction set is open, that's why I didn't use proper names. I actually considered using a completely different set of custom boolean properties like IsTalkAllowed for the most common ones, but I decided the version I went with required less properties and was more customizable.

It's not a bad idea to read the manual beginning-to-end (skimming over the scripting documentation, just getting a sense of the classes that exist): it's not that long. But there are definitely aspects of it that could be improved, and getting feedback from beginners is important input for that work. I'm hoping I'll have a chance to take a pass at an update soonish (meaning sometime this year).

I didn't know it was you who maintained it, sorry. Didn't want to offend. I did read the whole manual before I started a year ago, but something got filed in my brain under "advanced coding, be careful" and in the end you kinda know they are an option but can't think about it on the fly. That's why I said that references would be useful.
On a side note, there's one thing left to do with this code, which is making the menu the standard interaction with objects and characters too, but I guess I just have to expand the code in on_mouse_click to include a character and an object pointer. All the custom Action_n properties are already set up to be available to objects and characters too.

Oh, one more concern: I'm curious about your variable naming scheme...

Code: ags

vBoolAction2
vIntMouseActionX

You're using some weird version of Hungarian notation, with two prefixes: v- (not sure what this means) and the variable type (Int- or Bool-). In my opinion this is not good practice. Most coding style guides recommend against Hungarian notation (particularly system-Hungarian, where it's used to indicate the variable type). It's used in AGS for a few things (mostly built-in struct types, and enum options), but I definitely wouldn't use it for ints and bools.

I didn't even know it was called Hungarian notations 
If I'm allowed a bit of rambling, all my coding education is a few lessons on Turbo Pascal at school in the 90s on a prehistoric Commodore that still used 5 1/4 inches floppy disks (yes, in the 90s). Everything else (which you may as well say everything at all, because it taught me some really basic concepts like if-else checks, for/while loops and declaring variables and functions, but not much else; besides, who uses Pascal these days?) is basically self taught via occasional spurs of curiosity and creativity, like the current one.
I named variables this way because it seemed easier to remember for me what is what, especially for global variables that may be called all over the places. Why is it discouraged? What are the drawbacks?

[Snarky]

I didn't know it was you who maintained it, sorry.

It's not, but no one has really taken ownership of it since CJ retired (I think Crimson Wizard has been stuck doing the absolutely needed updates), so I've had it on my todo-list for a while.

[Snarky]

I'll look into enums, thanks. But can I set them as global variables? Can I set it up in the global scrip and then read them in the room hotspot function?

I don't think they can be set via the "Global Variables" pane, but you can declare an enum type in a script header, and it will be available to all scripts below it. You can also export and import an enum variable from the global script so it will be accessible to the room scripts, just like any other variable (bool, int, ...).

No, they change. In the current state of the project, the amount ranges from 0 (almost every hotspot[0] except two specific case) to at maximum 8, with an average of 4/5.
They change with each hotspot for a couple of reasons_
- since you can't talk to a fridge, for example, I think there's no sense in adding a "talk" option, so some interactions may not be available. This is what makes it hard to figure out what is in a specific spot. It is slightly confusing to have a series of Action_n properties, but since I code the interactions as I set up the hotspot and therefore the local values of each custom property, hopefully it's easy to remember which one is which.
- some of them indeed have custom names. Rather than a generic "use", it may be "turn on" for an electric appliance, "eat a snack" for the fridge, etc. In the case of the TV I put in the example post, it has two separate options for "whatch movie" and "whatch TV show", for example.

If they're 90% from some limited set, I would personally still use a list of meaningful names, and then have a few slots for "special actions". Something like:

Code: ags

struct PlayerAction {
  eActionInvalid = -1,
  eActionLook,
  eActionTalk
  eActionUse,
  eActionPickUp,
  // ...
  eActionCustom1,
  eActionCustom2,
  eActionCustom3
  // if you need more...
};

Then each hotspot could support any subset of them, probably with a set of flags as you initially planned.

I'm a big proponent of semantic coding: making the relationship between the code and the task as transparent as possible, by (among other things) using meaningful variable names, avoiding "magic numbers" in the code, etc. It's not just about writing the code, but making it readable after the fact.

I didn't even know it was called Hungarian notations 

I named variables this way because it seemed easier to remember for me what is what, especially for global variables that may be called all over the places. Why is it discouraged? What are the drawbacks?

1. In a strongly-typed language, it doesn't really provide useful information: the compiler should stop you from mixing up different types anyway (though admittedly the AGS compiler is fairly lenient in type-checking).
2. Also, the code editor has popups that tell you the type information.
3. With autocomplete, it's more you have to type before you get to the specific variable name.
4. It makes variable names longer, and buries the essential information within the name. This makes code harder to read.
5. (Not a problem with your variation of the notation: These prefixes can get both pretty obscure and impossible to pronounce, leaving you with variables like pszSrc or rgchBuf.)
6. Arguably, if the code is properly written you shouldn't have to worry about the type. The relevant methods should simply take arguments of the right type, so it should just make sense.

Prefixes can be useful if they organize related variables you might have to choose from (that's why I name all the enum values eActionXXX instead of just eXXX) or provide some information that is not given simply by the type (for example, view IDs show up in the code simply as integer constants, so I always use a V- prefix to distinguish them from other number constants). Or if you need two variables for related things that differ only by their type (e.g. a String and int representation of the same number). However, in these cases it is very often better to put the semantic "type" at the end of the variable name, rather than the front.

[dayowlron]

struct PlayerAction ?
didnt you mean enum PlayerAction ?

[Snarky]

Yes indeed.