Best Practices for organising GlobalScript.asc

[Cerno]

Hello all.

I have a question related to the discussion in this thread:
http://www.adventuregamestudio.co.uk/forums/index.php?topic=50052.0

It is about the old problem of organizing an ever-growing GlobalScript.asc file into managable chunks.
Since I would like to have all character-related scripting in one place, I decided to give each character their own script file.
In addition to that I have one script file for inventory items and one to collect all kinds of useful functions that I need all over the place.

So far the code has become much easier to look through, but I ran into a problem when it came to the action functions, e.g. cCharacter_Interact().
It seems that I can only properly call that function from GlobalScript.asc, so if I want to implement that function in CharacterScript.asc, I would have to write an intermediate function e.g. characterInteractHandler()

So apparently I have to do the following for each character interaction function and every character:

• Add the character interaction function cCharacter_Interact() into GlobalScript.asc
• Add the intermediate function characterInteractionHandler() into CharacterScript.asc
• Import the function in CharacterScript.ash
• Call characterInteractionHandler from cCharacter_Interact()

And I have to do this for each and every character interaction function (and each inventory item as well if I want to keep them in a separate script).

This seems pretty cumbersome and a lot of work for a seemingly simple task. Is there an easier way?

How do you guys organize your code? Keeping it all in GlobalScript seems very error-prone due to the sheer amount of lines of code in one file which is generally bad coding style.

[JSH]

For Kathy Rain, what I do is bypass the interact functions altogether by intercepting mouse clicks manually with on_mouse_click(MouseButton button) in GlobalScript.

To detect what the player wants to interact with I use:

Code: ags

<X>.GetAtScreenXY(Mouse.x, Mouse.y);

Where X is all types of interactible objects, ie, Hotspot, Object, InventoryItem and Character. Mouse.Mode is used to determine the currently selected cursor.

To retain normal behavior with event functions per room for Hotspots and Objects, I call

Code: ags

Hotspot.RunInteraction(Mouse.Mode);
Object.RunInteraction(Mouse.Mode);

For characters and inventory items I instead call extender functions defined in a separate script files:

Code: ags

function RunInteractionCustom(this Character*, CursorMode Mode);
function RunInteractionCustom(this InventoryItem*, CursorMode Mode);

This is what the function could roughly look like:

Code: ags

function on_mouse_click(MouseButton button)
{
  int MouseX = Mouse.x;
  int MouseY = Mouse.y;
  CursorMode Mode = Mouse.Mode;
  
  Character* pChar = Character.GetAtScreenXY(MouseX, MouseY);
            
  if(pChar != null)
  {
    pChar.RunInteractionCustom(Mode);
  }
  else
  {
    InventoryItem* pItem = InventoryItem.GetAtScreenXY(MouseX,MouseY);
    
    if(pItem != null)
    {
      pItem.RunInteractionCustom(Mode);
    }
    else
    {
      Hotspot* pHotspot = Hotspot.GetAtScreenXY(MouseX, MouseY);
      
      if(Hotspot != hotspot[0])
      {
        pHotspot.RunInteraction(Mode);
      }
      else
      {
        Object* pObject = Object.GetAtScreenXY(MouseX, MouseY);
        if(pObject != null)
        {
          pObject.RunInteraction(Mode);
        }
      }
    }
  }
}

In my case, it wasn't necessary to split character logic up into a file per character, just one file for dialogue and another for the rest of the interactions. But of course it depends on the complexity of your scripting and what kind of system you use for dialogue. Might be a good idea to do a character based separation on a case by case basis, since many characters most likely will have simple interactions with just a few lines of script.

Anyway, in each extender function I simply have a long list of if/elses separating all characters and inventory items respectively, and also make extensive use of CallRoomScript() to isolate room specific logic. By using an enum to create user friendly names for the values, they will autocomplete when you intercept them in the room scripts with on_call(int value).

Example CharacterInteractions.ash

Code: ags

enum eRoomScripts
{
    eRoomScriptRogerTalk,
};

function RunInteractionCustom(this Character*, CursorMode Mode);

Example CharacterInteractions.asc

Code: ags

function RunInteractionCustom(this Character*, CursorMode Mode)
{
    if(this == cRoger)
    {
        if(Mode == eModeTalk)
            CallRoomScript(eRoomScriptRogerTalk);
        else if(Mode == eModeLook)
            player.Say("Oh look, it's Roger!");
    }
}

And then in room_spaceship.asc:

Code: ags

function on_call(int value)
{
    if(value == eRoomScriptRogerTalk)
    {
        player.Say("So Roger, how do you like the space ship?");
    }
}

[Cerno]

Wow, JSH, thanks for your lengthy and insightful reply. I especially like the idea of bypassing the GUI-based scripting as I always thought that having the game logic split up between the GUI and the code to be one of AGSs less useful features.

I am currently not at home but when I return I'll check whether your approach can work for me. Definitely sounds interesting.

In the meantime maybe others can give some insight into their way of dealing with growing complexities in GlobalScript.asc.

There is one question I am still not certain about: Is it really true that I have to follow the 4 steps I initially described in order to move an action to a separate script or am I overlooking something?

[Snarky]

Yes. You can't move most event handlers in AGS (or "special functions", as AGS calls them), they're hardcoded to the global script/room script, so the best you can do is just have them call a function from another script, to which you delegate all the actual work. And since that function is in another script, it does need to be imported. So the way you're doing it is right, unless you want to completely bypass the on-interaction events, like JSH proposes.

But as JSH points out, you probably don't need separate script files for every single character, so you shouldn't have that many functions to declare/import in the headers. And multiple events can have the same handler, so you can create a generic character_Interact() function in the global script, and set it as the interaction event handler for all the different characters. (Copy-paste the name into the event field.) Then you can simply check which character was clicked, and call the appropriate function. Better than having dozens of nearly-identical, nearly empty functions.

The idea of bypassing interaction events completely and just interpreting the mouse clicks yourself also offers a slightly different approach, because you CAN in fact have multiple on_mouse_click() functions. So each characterscript file could have an on_mouse_click() function that just checks whether its character is clicked, and if so runs the appropriate interaction. (The naive approach of just calling Character.GetAtScreenXY() over and over would probably be quite inefficient, but there are some pretty simple ways to optimize it.)

[Cerno]

A few thoughts/questions on your ideas:

For Kathy Rain, what I do is bypass the interact functions altogether by intercepting mouse clicks manually with on_mouse_click(MouseButton button) in GlobalScript.

Did you experience any deviations from the standard AGS behaviour? For instance, what happens if an object lies in front of a character, then you click the object and call character.GetAtScreenXY? Will it return the character or nothing (since the character is "invisible" where it is occluded by the object)? In other words: Do I have to re-implement any logic that the game usually handles internally?

But as JSH points out, you probably don't need separate script files for every single character, so you shouldn't have that many functions to declare/import in the headers

Um, isn't the number of imports dependent on the number of functions, not the number of script files? I would still need one import per function, no matter how many scripts I have, right? Unless you are talking about your next point:

And multiple events can have the same handler, so you can create a generic character_Interact() function in the global script, and set it as the interaction event handler for all the different characters. (Copy-paste the name into the event field.) Then you can simply check which character was clicked, and call the appropriate function. Better than having dozens of nearly-identical, nearly empty functions.

I really liked that idea at first, since it requires much less custom code than JSH's approach, until I realized that this would actually cause AGS to look up what was clicked on two times: Once internally to call the correct action and then again a second time when I manually call GetAtScreenXY(). I can't do that, it breaks my black little coder's heart.

The naive approach of just calling Character.GetAtScreenXY() over and over would probably be quite inefficient, but there are some pretty simple ways to optimize it.

Assuming I went with JSH's approach, would it really be so inefficient? If I use regular actions, AGS has to look up what was clicked, so if I don't implement an action but instead use the GetAtScreenXY() function, that would be the same thing right? Also I don't quite understand what you meant by the function being called over and over again. The on_mouse_click() function is only called once on each click and then we would have one GetAtScreenXY for each object type (hotspot, character, etc.) Is that really a lot more inefficient than the way AGS handles things internally? Sorry, I don't have any idea what AGS looks like under the hood. So if it turns out that it really is inefficient, could you give a hint on how to optimize?

In general, I wonder whether anyone ever discussed a solution for this common problem: Can we find a hassle-free way of splitting up the GlobalScript into manageable chunks? Some hare-brained ideas came to my mind:

• Add an INCLUDE command to the AGS syntax that pastes the contents of a text file into the script (similar to a preprocessor #include in C).
• Define subviews on a script (e.g. by using pragma-like structures "pragma SPACEMAN_CODE_BEGIN [enter all spaceman functions here] pragma SPACEMAN_CODE_END". Everything within the pragma block is accessible in the project tree as a separate item, and opens a separate editor which would work like a subview on the whole script. Might be ugly to code though, since it would require updating all overlapping subviews simultaneously.
• Similar to the idea above but probably easier to implement: Add bookmarks to script files than can be accessed via the project tree.
• The most obvious from a user's perspective: Make it possible to implement actions in any script, not only in GlobalScript.asc (depending on whether that is even possible within the existing means of AGS. There may be a good reason why this hasn't been implemented already.)

[Snarky]

Did you experience any deviations from the standard AGS behaviour? For instance, what happens if an object lies in front of a character, then you click the object and call character.GetAtScreenXY? Will it return the character or nothing (since the character is "invisible" where it is occluded by the object)? In other words: Do I have to re-implement any logic that the game usually handles internally?

You do, but it's pretty simple. First you should call GetLocationType(x,y), and then the appropriate GetAtScreenXY(x,y) for hotspot/object/character.

Um, isn't the number of imports dependent on the number of functions, not the number of script files? I would still need one import per function, no matter how many scripts I have, right? Unless you are talking about your next point

I was. I meant that you'd have one common interaction handler, calling one function from a shared character script, which would then branch depending on the particular character that is clicked.

I really liked that idea at first, since it requires much less custom code than JSH's approach,

I'm not sure it does. It would just make it easier to split it across multiple different script files.

until I realized that this would actually cause AGS to look up what was clicked on two times: Once internally to call the correct action and then again a second time when I manually call GetAtScreenXY(). I can't do that, it breaks my black little coder's heart.

You can't get around that. JSH's approach also depends on it. It's either that or doing it by manually linking every interaction handler to a separate script-file function, imported individually.

The naive approach of just calling Character.GetAtScreenXY() over and over would probably be quite inefficient, but there are some pretty simple ways to optimize it.

Assuming I went with JSH's approach, would it really be so inefficient? If I use regular actions, AGS has to look up what was clicked, so if I don't implement an action but instead use the GetAtScreenXY() function, that would be the same thing right? Also I don't quite understand what you meant by the function being called over and over again. The on_mouse_click() function is only called once on each click and then we would have one GetAtScreenXY for each object type (hotspot, character, etc.) Is that really a lot more inefficient than the way AGS handles things internally? Sorry, I don't have any idea what AGS looks like under the hood. So if it turns out that it really is inefficient, could you give a hint on how to optimize?

The key point is that you can have multiple on_mouse_click() functions (one in each script), and they'll all be called sequentially when you click... unless one of them claims the event, which stops any ones further down from seeing it. So the simplest way to code it would be to have an on_mouse_click() function in every character script, and just do if(GetLocationType(mouse.x, mouse.y) == eLocationCharacter && Character.GetAtScreenXY(mouse.x,mouse.y)==cWhatever) in every script. But of course that's horrendously inefficient, since you're looking up the location twice for every character. That's what I meant.

But you could have one script on top with an on_mouse_click() function that does the lookup and sets a global variable, and then the various character script on_mouse_click() functions just check that global variable to see whether they've been triggered.

[Cerno]

Thanks for the clarification Snarky.

So here is what I finally did:

For the characters, I went with my initial approach, since I wanted each character in its own script as it seems much cleaner that way.
Since I only have a handful of characters, it wasn't too much effort to forward their functions to handler functions imported in the corresponding character scripts.

For the inventory items, I chose your solution and gave each item the same two actions: item_Look and item_UseInv, implemented like this:

Code: ags

function item_Look() { 
  InventoryItem* item = InventoryItem.GetAtScreenXY(mouse.x, mouse.y);
  itemLookHandler(item); 
}

function item_UseInv() { 
  InventoryItem* sourceItem = player.ActiveInventory;
  InventoryItem* targetItem = InventoryItem.GetAtScreenXY(mouse.x, mouse.y);
  itemUseInvHandler(sourceItem, targetItem); 
}

The functions itemLookHandler and itemUseInvHandler are implemented in one additional script file.

When implementing the itemUseInvHandler function, I realized something really neat that bothered me a lot before:
I can finally make item combining automatically symmetric. I no longer have to make sure manually that combining the hamster with the tiny sweater has the same effect as doing it in the opposite order.

I did it like this:

Code: ags

struct ItemPairCompFunctor {
  InventoryItem* item1;
  InventoryItem* item2;
  import bool compare(InventoryItem* item1cmp, InventoryItem* item2cmp);
  import function set(InventoryItem* item1, InventoryItem* item2);
};

bool ItemPairCompFunctor::compare(InventoryItem* item1cmp, InventoryItem* item2cmp) {
  return ((this.item1 == item1cmp && this.item2 == item2cmp) || (this.item1 == item2cmp && this.item2 == item1cmp));
}

function ItemPairCompFunctor::set(InventoryItem* item1, InventoryItem* item2) {
  this.item1 = item1;
  this.item2 = item2;
}

So my final function itemUseInvHandler looks like this (albeit with a few more cases):

Code: ags

function itemUseInvHandler(InventoryItem *sourceItem, InventoryItem *targetItem){
  ItemPairCompFunctor comp;
  comp.set(sourceItem, targetItem);
  
  if (comp.compare(iHamster, iTinySweater))  {
    player.AddInventory(iDryHamster);
    player.LoseInventory(iHamster);
    player.LoseInventory(iTinySweater);
  }
  else  {
    int what = 5; //INVENTORY ITEM
    int type = 3; // USE_INV
    defaultEvent(what, type); // a handler for unhandled_event()
  }
}

I am quite happy with the results.

Thanks a lot Snarky and JSH for the extensive help!

[Khris]

Disclaimer: I didn't read the entire thread so far.

You can optimize this further, because if you put "item_Look" next to every single inventory item's "look at", you can simply intercept the click right in on_mouse_click. Example:

Code: ags

  else if (button == eMouseRightInv) {
    if (player.ActiveInventory == null) itemLookHandler(inventory[game.inv_activated]);
  ...

And handling "use inv on inv":

Code: ags

  else if (button == eMouseLeftInv) {
    if (player.ActiveInventory != null) itemUseInvHandler(player.ActiveInventory, inventory[game.inv_activated]);
  ...

That way you can leave the inventory items' event tables empty.

Bonus :

Code: ags

int sum, diff;
bool CheckInvCombo(InventoryItem *a, InventoryItem *b) {
  return a.ID + b.ID == sum && Math.abs(a.ID - b.ID) == diff;
}

void itemUseInvHandler(InventoryItem *active, InventoryItem *target) {
  if (active == target) ...  // do nothing?
  sum = active.ID + target.ID;
  diff = Math.abs(active.ID - target.ID);

  if (CheckInvCombo(iHamster, iTinySweater)) ...

[Cerno]

Wow, thanks!

The top one would have saved a lot of copy-pasting

It seems the bottom one is pretty much what I did except you used a clever sum and diff trick while I compared the items directly.
Both our approaches should have the same runtime, right?

Anyway, thanks for your input.

[Gurok]

I think yours also makes it possible to reuse ItemPairCompFunctor. Not saying Khris' isn't efficient though.

Another way is to sort the two inventory item IDs, then store them in a single integer. Something like firstItem.ID * Game.InventoryItemCount + secondItem.ID. Inventory items are so few in number, that you'd probably be fine with that. You could have an array of all the combinations you care about and precalculate them in a game_start. Maybe use an enum to keep track of the indices of the array. You can then do all of your comparisons with a simple ==.

I rather like your OO approach though, Cerno. Some semantic things:
- I think ItemPair is fine. AGS doesn't really have support for function pointers or function objects right now. I understand it acts like a functor though
- Compare is being used in the opposite way to what you'd normally expect. Maybe Matches is a better name? Compare functions are normally non-zero if the values don't match

Me, I don't encapsulate the InventoryItem * pointers at all and just leave them in the module (asc). Inventory_Left and Inventory_Right. Then, I just have a simple compare function that looks like Cerno's. The variables are global to other functions within the same module, but hidden from other modules. As I'm generally pretty strict about one class per module, a struct for the pair felt like overkill.

In addition to that I have one script file for inventory items and one to collect all kinds of useful functions that I need all over the place.

...

So apparently I have to do the following for each character interaction function and every character:

• Add the character interaction function cCharacter_Interact() into GlobalScript.asc
• Add the intermediate function characterInteractionHandler() into CharacterScript.asc
• Import the function in CharacterScript.ash
• Call characterInteractionHandler from cCharacter_Interact()

...

How do you guys organize your code? Keeping it all in GlobalScript seems very error-prone due to the sheer amount of lines of code in one file which is generally bad coding style.

I do separate files for each inventory item, each character and each GUI (as well as single files for classes). My global script is a series of what I call stubs, like this:

Code: ags

void cReceptionist_Look() { CReceptionist.Look(); }

I guess I pretty much follow the method you outlined in your first post. I don't find it that cumbersome. I know it doesn't suit everyone, but GUI-based script bindings are fine by me. My reasoning is partly that I prefer to reinvent as little as possible. Sure, you can write custom mouse interaction code. You can also rewrite the pathfinding. The less you rewrite, the smaller the scope of your testing.

You can make something pretty close to a template for a person / item. I use structs as a bit of a namespacing tool too, as you can see with the Look method. That way, you can just dump the template in, find/replace and the most painful part is done.

Regarding the #pragma thing, there's support for:

Code: ags

#region Your Name Here

and

Code: ags

#endregion

In 3.4.x, at least, I think. It doesn't create a sub-unit that can be managed in the project tree yet, but that's a natural extension, as you mentioned. 3.4 might also appeal to you with switch/case. It's possible to switch(true) and case comp.compare(iHamster, iTinySweater), which is neat (I think).

[Cerno]

Thanks for the input Gurok.

I renamed the function as per your advice.

So, do you really use one script file per inventory item? Isn't that an awful lot of effort? Clicking through all the menus generate the scripts must be a nightmare. Also doesn't your Script subtree get immensely long and hard to scroll through?

Reading your reasoning about why you use the internal AGS capabilities made me realize why exactly I finally made my decision to do it my way and not went with JSH's idea (although it sounds pretty compelling). Having to debug custom code in an environment that I am not fluent in and a framework whose inner workings I know next to nothing about was just too risky for me. Forwarding the actions to custom scripts felt like an ideal compromise between reducing effort, keeping a solid structure and minimising potential unpleasant surprises down the road.

Finally, from what you write, AGS 3.4 seems to be right up my alley
When will it be out? I don't feel lucky enough to start using the beta right now
Care to describe a bit more what the #region syntax does?

[Gurok]

So, do you really use one script file per inventory item? Isn't that an awful lot of effort? Clicking through all the menus generate the scripts must be a nightmare. Also doesn't your Script subtree get immensely long and hard to scroll through?

Folders make it pretty easy to navigate. And like I said, I've opted for more of a template system, so I can copy + paste most stuff for new inventory items.

Finally, from what you write, AGS 3.4 seems to be right up my alley
When will it be out? I don't feel lucky enough to start using the beta right now
Care to describe a bit more what the #region syntax does?

I don't know when the final release would be out. I'd recommend holding off until at least the next build.

The only effect #region has right now is to create a section used for code folding. It's still pretty neat. You can fold up all but what you're working on. It was backported from a fork that Alan v.Drake wrote a long time ago.