Do's and Don'ts of HSL

From HEWIKI
Jump to: navigation, search

Contents

Do's and Don'ts of HSL is intended as a central location to see best practices and important programming concepts that will help the HSL scripter. Its creation is the result of short reviews done by an in-house programmer with other experienced in-house scripters.

Please also see:

Associations

Build node hierarchies for searching efficiency

When dealing with persisted nodes, avoid directly associating with the root node unless by a unique association type. Looping through all of the persisted nodes looking for a particular class, could get slow. Organizing the nodes into a hierarchy could boost performance significantly when dealing with hundreds of nodes.

Auto-Conversion

Avoid Auto-Conversion of Literals

Auto-conversion is less efficient than using the correct literal, and defeats compile time validation. For example, auto-converting a string literal to an enum value or script identifier is bad.

Example 1: Bad

    s as scriptref = "BaseAreaScript"

Example 2: Good

    s as scriptref = BaseAreaScript

Example 3: Bad

    // where areaType is a field of type enum AreaTypes
    if n.areaType = "RESIDENCE"
      // do stuff
    .

Example 4: Good

    // where areaType is a field of type enum AreaTypes
    if n.areaType = RESIDENCE
      // do stuff
    .

Trust in Auto-conversion

Avoid using functions like itos() and stoi() when auto-conversion will do the job. In fact, since auto-conversion was added, there have been cases where the wrong function was used, producing functioning code that inefficiently converts 3 times. (As a note, to get a String from a Node Ref or vice versa, there are function in the strutils script on both client and server which should be used, which are stoid and idtos. If you use stoi or itos on NodeRefs, you'll lose numbers because IDs are 64-bit unsigned while Integers are signed.)

Example 1: Bad

    // converts integer to string to integer to string
       s as string = stoi(anInteger)

Example 2: Good

    // will only convert once
       s as string = anInteger

Functions

Pass/Return "Big" Arguments by Reference where Possible

Avoid making functions that return large lists or classes. It is more efficient to pass the list or class as a reference argument. The first example below requires that the whole LookupList be copied between source and target when it could have operated directly on the target.

Example 1: Bad

    function ParseProperties(propStr as String) as LookupList indexed by String of String

The above function has to make a copy of the LookupList to return it to the caller.

Example 2: Good

    function ParseProperties(propStr as String, results references LookupList indexed by String of String)

The above passes in an lookuplist which is simply "filled in" by the function, which is much faster (in HSL or any programmer language).

Using functions to return complex constants is also inefficient. See Don't Copy Variables Unnecessarily under Variables and Fields for examples.

Shared function use

Shared functions can be more difficult to change, so should only be designated Shared when needed. Its primary use in HSL is to simulate traditional object-oriented programming.

Use specific named parameters

Avoid passing arguments through containers rather than its own argument. It is less clear for someone attempting to call the function and less efficient.

Example 1: Bad

    function lessClear( args as lookupList indexed by string of string)
      MsgPlayer( args["who"], "", "hello")
    .

Example 2: Good

    function moreClear( who as string)
      MsgPlayer( who, "", "hello")
    .

Glomming

Associations make better markers than Glommed classes

Prefer associations to class glomming when simply keeping track of a selected node or nodes. In the first example below, a preferred character is kept track of by glomming a class onto that character, which requires looping through all characters. But in the second example, the preferred character is kept track of using an association that is faster to query, and in this case, defined to be unique, thus preventing unexpected behavior.

Example 1: Bad

  function GetPreferredCharacter( accountNode as NodeRef) as NodeRef
    assert( accountNode != None, "accountNode was None")
    foreach a in QueryAssociation( accountNode, "base_hard_association", 0)
      if a.target is kindof preferred_character
        return a.target
      .
    .
    return None
  .
  function RememberPreferredCharacter( charNode as NodeRef, accountNode as NodeRef)
 
    prev as NodeRef = GetPreferredCharacter( accountNode)
    if prev = charNode
      // it is already the one
      return
    .
    if prev != None
      removeClassFromNode("preferred_character", prev)
    .
    glomClass("preferred_character", charNode)
  .

Example 2: Good

  function GetPreferredCharacter( accountNode as NodeRef) as NodeRef
    assert( accountNode != none, "accountNode was None")
    al as List of Association = queryAssociation( accountNode, "preferred_character", None)
    if al.length = 0
      return None
    else
      assert(al.length = 1, "association was supposed to be +uniqueTarget")
      return al[1].target
    .
  .
  function RememberPreferredCharacter( charNode as NodeRef, accountNode as NodeRef)
 
    // preferred_character association has +uniqueTarget, so adding this will replace the old one  
    AddAssociation( accountNode, "preferred_character", charNode)
  .

Glomming and multiple inheritance can have unintended consequences

Watch out for the same field being used for more than one purpose. This could happen subtly if more than one class uses a field name, and they end up on the same node via inheritance or glomming. It is best to make unique field definitions for each conceived purpose.

Remember, if a field appears more than once in the class hierarchy (or hierarchies) attached to a Node, the field itself exists only once on the node. It is important to name your fields verbosely and specifically as they exist in a single flat name space.

Logic

Account for all branches in logic statements

Be careful when adding WHERE statements to consider what should happen in the case the WHERE doesn't match. This also applies to WHEN and IF statements. Silently ignoring the situation can make bugs that are hard to find.

Example 1: Good

  where n
    is kindof _playerAccount
    .
    default
      ScriptError("Good to report this if you think it should never happen")
    .
  .

Avoid duplicate code

Avoid duplicating the same or very similar code in different places. Refactor bad code by extracting it into a function.

Example 1: Bad

function demo1()
    root as noderef of class example = GetRoot()
    if stuff
        root.demoAmount = root.demoAmount + 1
        if root.demoAmount > root.demoLimit
            root.demoAmount = root.demoLimit
        .
    else
        root.demoAmount = root.demoAmount + someAmount
        if root.demoAmount > root.demoLimit
            root.demoAmount = root.demoLimit
        .
    .
.

Example 2: Good

function demo1()
    if stuff
        demoAdd(1)
    else
        demoAdd(someAmount)
    .
.
 
function demoAdd( amount as integer)
    root as noderef of class example = GetRoot()
    root.demoAmount = root.demoAmount + amount
    if root.demoAmount > root.demoLimit
        root.demoAmount = root.demoLimit
    .
.

Be aware of actions that generate script callbacks

Avoid stopping and restarting or pausing and unpausing timers unnecessarily. Each operation attempts a script callback and that you may not intend or need.

Example 1:

Area.AreaSpinDownTimer.stop()
Area.AreaSpinDownTimer.start()

Example 2: Good

Area.AreaSpinDownTimer.elapsedTime = 0:00:00

Be clever with math to avoid slow, error-prone logic

A common approach by novice programmers is to use conditional logic to determine a value, when a direct computation or table lookup would be more efficient and less prone to error.

Example 1: Bad

    if strength < 10
      bonus = 0
    else if strength > 10 and strength < 20
      bonus = 1
    else if strength >= 20 and strength < 30
      bonus = 2
    else
      bonus = 3
    .

Example 2: Good

    bonus = strength / 10
    if bonus > 3
      bonus = 3
    .

Break up complex code into smaller functions

Lengthy functions are hard to follow. Nested conditional blocks that cannot be seen all at once in the editor are especially bad. Where practical, break code up into smaller functions.

Example 1: Good

  public function start()
    println("Got CCSModularSystem:start")
    destroyCurrent()
    Setup()
    LaunchNextStep()
    SetupCamera()
  .

Check and use utility functions

Often times other coders on your team may have already created functions or methods that are useful to you. Be aware of these by regularly reviewing the utilities in the Utilities folder of the Script Organizer (or wherever your team stores utility functions or methods)

Don't forget to break out of loops

When finding a single element in a list, be sure to break out of the loop once it's found, otherwise you waste CPU time continuing the loop when it's unneeded.

Example 1: Good

    found as boolean = false
    foreach elmt in list
      if elmt = theOne
        found = true
        break
      .
    .

Lookup Lists

Ensure That LookupList Keys Are Unique

Be careful when storing information in a LookupList with more than one way of determining the index value to ensure that there is no conflict between them. For example, indexing by either a character name or an account name wouldn't be correct if two different people used the same name - one as their account and one as their character.

Use the Has Operator to Check a LookupList for a Key

Attempting to reference a non-existent key in a LookupList will add that key to it, so when you simply want to check for the existence of a key, you must use the HAS operator.

Example 1: Good

  // where buddies is a LookupList and name is a string
  if n.buddies has name
    // do stuff
  .

NodeRefs and ScriptRefs

Compare ScriptRefs Against None Keyword, Not ""

Don't compare a Script Ref with an empty string. It depends on things that may change, and is not efficient.

Example 1: Bad

  function doIfSet(s as ScriptRef)
    if s != ""
      s:engage()
    .
  .

Example 2: Good

  function doIfSet(s as ScriptRef)
    if s != None
      s:engage()
    .
  .

Evaluate NodeRefs Against 'None', Not '0'

When checking if a Node Ref refers to a valid node, it is safest to compare it to the keyword 'None'. You may be led to use '0' because some error messages will give the node id of zero when attempting to use a Node Ref that doesn't refer to a valid node.

Example 1: Bad

    function demo(n as noderef of class demo)
      if n != 0
        n.demoField = "demo"  // could cause script error
      .
    .

Example 1: Good

    function demo(n as noderef of class demo)
      if n != None
        n.demoField = "demo"  // should never cause script error
      .
    .

Prototypes

Always-used, global data should be stored in prototypes

Don't create functions that return complex data objects if the data is going to be used everywhere. Prototypes are loaded into memory for every area, and for every run of HeroBlade for client prototypes, so can be used to easily access information that doesn't change.

Example 1: Bad

  potion as NodeRef of Class BasePotion = MakePotion()
 
  function MakePotion() as NodeRef of Class BasePotion
    potion as NodeRef of Class BasePotion = CreateNodeFromClass("QuasiStrength")
    potion.abilityName = "QuasiStrength"
    potion.curCharges = 5
    potion.maxCharges = 5
    // ...
    return potion
  .

Example 2: Good

  potion as NodeRef of Class BasePotion = CreatePersistedNodeFromPrototype("QuasiStrength")

Don't use prototypes to store data that is local or infrequently used

Server prototypes are loaded into memory for every area. If the data you are trying to store isn't used throughout the whole game, for example, it only applies to the CCS area, then it probably shouldn't be stored in a prototype.

Similarly, client prototypes are loaded into memory for every run of the client. If the data you are trying to store isn't used every run, it probably shouldn't be stored as a prototype. Consider using the Repository Data File mechanism instead, if the data can be used asynchronously.

Readability

Be aware of when to make code efficient versus readable

Simplicity and clarity of code is often sacrificed in the name of efficiency. It is a judgement call, but consider the alternatives before making complicated or tricky code. If the code is heavily used, such as inside loops, efficiency becomes more important. On the client, the desired responsiveness is dependent on what's going on. For example, the player waiting a moment for the response from a click in the character system is acceptable; however, the same delay during combat could be fatal.

Follow standard conventions for naming fields and classes

Although the naming conventions described at Definition Names were not created until well after scripting on Hero's Journey began, you should always follow them and strive to correct improperly named fields in systems over which your purview extends.

Using TimerIntervals enhances readability

Use the Time Interval type for times instead of integers or floats. It avoids confusion between seconds, milliseconds, etc.

Example 1: Bad

    n.startDelay = 5  // is that seconds, milliseconds, minutes?

Example 2: Good

    n.startDelay = 00:05:00.0

Remote Calls

Don't blow the bandwidth budget

Remote calls to a client count as bandwidth used and have top priority, so sending too much can degrade replication. See Bandwidth Control.

Asynchronous System Utilities require checks for conflicts

When building an asynchronous system, be sure to deal with the operation being cancelled or superceded. Good and bad examples can be found in the Travel script.

Don't forget the Remote and Untrusted modifiers

Main page: Function/method modifiers

Remember to use the Remote modifier on remotely called functions and methods, and Untrusted for anything that may be called from the client.

Example 1: Bad

  public function example(rci as string)
    // stuff
  .

Example 2: Good

  remote function example(rci as string)
    // stuff
  .

Strings

Don't pack strings, use multiple fields

For best performance and readability, store values in separate fields instead of packed together such as in a string.

Example 1: Bad

    // This would be better as separate fields
    SplitBy(animation.GUIanimationArgs, ",", args)
    if(args.length != 10)
      println("Invalid number of arguments for the interpolate animation, arguments=" + animation.GUIanimationArgs)
      return false
    else
      // do stuff ...
    .

Know how to use and read SubString()

The third argument to SubString() is the number of characters to be returned not the ending position. Some code is misleading because it passes values for the third argument that are known to exceed the length of the string. In example 1 below, SubString() cannot possibly return input.length characters, but it is valid to obtain all of the characters following the starting position.

Example 1

    data = SubString(input, 2, input.length)

In example 2, the comparison will evaluate as true only if the input string is exactly 3 characters long and the 3rd character is ";". If it is greater than 3 characters, the comparison will evaluate as false.

Example 2

    if Substring(input,3,3) = ";"
      // do stuff
    .

Watch for string comparison case sensitivity

Comparing string values in HeroScript is case-sensitive. Consider forcing the string ToUpper or ToLower in situations where it may be a problem.

Example 1: Good

  function checkName(s as string)
    if ToUpper(s) = "BOB"
    .
  .

System Variables

Avoid using script execution system variables

Things like SYSTEM.EXEC.CPULIMIT and SYSTEM.EXEC.RUNTIME should be set at the earliest location in a script call possible, because setting them later in the script's execution may actually lower the limit set earlier by another function call. While some situations may call for the values of these variables to be increased, you may wish to check with your Technical Lead before doing so.

Example 1: Good

    SYSTEM.EXEC.CPULIMIT = SYSTEM.EXEC.CPULIMIT + 0:00:30

Restore System Variables Once Edits are Done Being Used

SYSTEM.EXEC.CALLSTACKLIMIT should be saved and restored when needed to be set. The intent is that you should only make an exception for the specific section of code that requires it, not for an entire script process. Again, yoy may wish to check with your Technical Lead before editing these kinds of variables.

Example 1: Good

    saveStackLimit as integer = SYSTEM.EXEC.CALLSTACKLIMIT
    SYSTEM.EXEC.CALLSTACKLIMIT = SYSTEM.EXEC.CALLSTACKLIMIT + 10
    // call stuff
    SYSTEM.EXEC.CALLSTACKLIMIT = saveStackLimit

Use System Variables rather than old hacked values

Be sure to use SYSTEM.INFO.WORLDANCHOR instead of setting a Node Ref to 999999.

Tools

Use the GUI Editor

When making GUIs, you should put as much as you can in the GUIXML. This allows you and others to use the GUI Editor and is more efficient at runtime.

User Input

Add sanity checks to user input

Care should be taken to limit making requests to the server in response to client input events. People will rapidly click or move the mouse over something and generate a lot of events, so doing things like only allowing a user to perform one search in an auction system may be a good thing. In this case you would disable the rest of the auction interface until the search request came back from the server, or achieved some timeout limit.

Don't allow user input to cause script errors

Care should be taken when processing user input so that problems are reported to the user rather than producing script errors. In the following example, if there was no semicolon in the user's input, a script error would be thrown, so the code should validate the input before processing it.

Example 1: Bad

    SplitBy(input, ";", commandLine)
    SplitBy(commandLine[1], " ", commands)
    SplitBy(commandLine[2], "~", propertyPairs)  // if there was no semicolon, this will script error

Matching input

Prefix-matching techniques in commands may be awkward to maintain. It is also flawed in that a prefix that happens to be a substring of a prior possible match will prevent the prefix from matching.

Example 1: Bad

    //               1234567890123456789012345678
    when FindString("lounge^set^office", toLower(parsed[2]))
      is 1
        // do lounge
      .
      is 8
        // do set
      .
      // ...

A partialmatch statement should replace this, as in example 2.

Example 2: Good

    partialmatch toLower(parsed[2]))
      is "lounge"
        // do lounge
      .
      is "set"
        // do set
      .
      // ...

Variables and Fields

Avoid duplicating literals

Avoid duplicating literals like numbers and string values. Since HeroScript doesn't currently have constants, make a function that simply returns the value.

Example 1: Bad ? if occurs more than once

    rmc.toInstance.AreaID = 1234

Example 2: Good

    rmc.toInstance.AreaID = TrackingArea()
    function TrackingArea() as ID
      return 1234
    .

Don't copy variables unnecessarily

Avoid making variables that are merely copies of complex fields or list elements. There is a tradeoff here between the cost of making the copy and the cost of repeated node field or list element lookup. Consider passing the field or list element to another function where it can be passed by reference.

Example 1: Bad

    temp as list of id = n.friends  // expensive if list is long
    foreach friend in temp
    .

Example 2: Good

    foreach friend in n.friends
    .

Example 3: Good

    // NPCDrivenState is a field of type LookupList indexed by id of class NPCDriveInfo
 
    function example()    
      // lookup once and pass as reference
      CalculateEndFacing( state.NPCDrivenState[npc] )
    .
    function CalculateEndFacing( info references Class NPCDriveInfo)
      v as Vector3 = info.npcDriveEndPosition - info.npcDriveStartPosition
      info.npcDriveEndRotation.x = 0
      info.npcDriveEndRotation.y = ArcTangent( -v.x, -v.z)
      info.npcDriveEndRotation.z = 0
    .

Use internally consistent field naming conventions

Good naming conventions can avoid a lot of problems like the same field being used for two different purposes. See the _FxSystemClassMethods client script for an example of doing this correctly.

Personal tools
Namespaces
Variants
Actions
Navigation
Toolbox