Talk:SimpleDialogMenuSystem

From Second Life Wiki
Revision as of 15:53, 12 December 2013 by Omei Qunhua (talk | contribs) (Agree)
Jump to navigation Jump to search

List of Changes Made

List of line changes from top to bottom:

  • float Timeout= 60; TO float Timeout = 60.0;
    • To make it a true float value.
  • giveDialog(key ID, integer pageNum) { TO giveDialog(key ID) {
    • pageNum was declared as a global variable and is not needed as a local in the function.
  • llDialog(ID, "Page "+(string)pageNum+"\nChoose one:", buttons, channel_dialog); TO llDialog(ID, "Page "+(string)pageNum+"\n"+msg, buttons, channel_dialog);
    • What is the point in having string msg declared at the top if not using it?
  • llSetTimerEvent(0); TO llSetTimerEvent(0.0);
    • To make it a true float value.
  • giveDialog(ToucherID, pageNum); TO giveDialog(ToucherID);
    • This refers to the ones in the listen section. As stated above, pageNum was declared as a global variable.
  • llListenRemove(listen_id) TO CancelListen();
    • In the timer section. Might as well.

I also moved one line:

  • llSetTimerEvent(Timeout);

From touch_start to giveDialog. Reason being, if someone chose the "Next" or "Prev" page, it would give them the dialog again, but would not reset the timer. In other words, if it took someone 50 seconds to decide to push the "Next" page button, they would then only have 10 seconds to choose a button. If they decided to go to the next page again, or go to the previous page, that would leave them even less time. I thought about doing the same for:

  • listen_id = llListen( channel_dialog, "", ToucherID, "");

As if someone were to push the "Back" button, it would remove the listen and would have to be declared again if they were redirected to another menu. However, I realized the creator/user of the script may not use the giveDialog function to give them the previous menu, and decided not to change this.

L1ghtsh0wz3n Resident 10:44, 9 November 2013 (PST)

It's got other bugs, if you spam click it, triggering multiple touch_start events, it will leak listens to the point you crash the script. pageNum shouldn't have been made a global, it should have remained a local but the entire design leaves much to be desired and your solution is valid. The script needs to be rewritten. Keep up the good work. -- Strife (talk|contribs) 11:28, 9 November 2013 (PST)

Suggested Rewrite

What would people think of this possible version? It uses state changes to preclude other users triggering touches while one user is accessing the dialogs, and the state change is also used to kill any/all open listeners. Omei Qunhua 14:25, 12 December 2013 (PST) <lsl>

// Multi-Page Dialog Menu System

// Omei Qunhua December 2013

integer gActionsPerPage = 9; // Number of action choice buttons per menu page (must be 1 to 10, or 12) list gListActions = ["A","B","C","D","E","F","G","H","I","J","K","L","M","N","O","P","Q","R","S","T","U","V","W","Y","Z"];

// ========================================================================================================

integer gTotalActions; integer gPage; // Current dialog page number (counting from zero) integer gMaxPage; // Highest page number (counting from zero) integer gChan; // Channel used for dialog communications. key gUser; // Current user accessing the dialogs

BuildDialogPage(key user) {

   // Build a dialog menu for current page for given user
   integer start = gActionsPerPage * gPage;       // starting offset into action list for current page
   // set up scrolling buttons as needed
   list buttons = [ "<<", " ", ">>" ];
   if (gActionsPerPage == 10)           buttons = [ "<<", ">>" ];
   else if (gActionsPerPage > 10)      buttons = [];          // No room for paging buttons
   integer x;
   for (; x < gActionsPerPage; x++)
   {
       if ( (start + x)  < gTotalActions)
       {
           buttons += llList2String(gListActions, start + x);
       }
   }
   llDialog(user, "\nPage " + (string) (gPage+1) + " of " + (string) (gMaxPage + 1) + "\n\nChoose an action", buttons, gChan);
   llSetTimerEvent(60);              // If no response in time, return to 'ready' state

}

default {

   state_entry()
   {
       gTotalActions = (gListActions != [] );        // get length of action list
       // Validate 'ActionsPerPage' value
       if (gActionsPerPage < 1 || gActionsPerPage > 12 || gActionsPerPage == 11)
       {
           llOwnerSay("Invalid 'gActionsPerPage' - must be 1 to 10 or 12");
           return;
       }
       // Compute number of menu pages that will be available
       gMaxPage = (gTotalActions - 1) / gActionsPerPage;
       if (gActionsPerPage == 12)
       {
           gMaxPage = 0;
           if (gTotalActions > 12)
           {
               llOwnerSay("Too many actions in total for this ActionsPerPage setting");
               return;
           }
       }
       // Compute a negative communications channel based on prim UUID
       gChan = 0x80000000 | (integer) ( "0x" + (string) llGetKey() );
       state ready;
   }

} state ready {

   touch_end(integer total_number)
   {
       gUser = llDetectedKey(0);
       state busy;                                    
       // Changing state sets the application to a busy condition while one user is selecting from the dialogs
       // In the event of multiple 'simultaneous' touches, only one user will get a dialog
   }

} state busy {

   state_entry()
   {
       llListen(gChan, "", gUser, "");                // This listener will be used throughout this state
       gPage = 0;
       BuildDialogPage(gUser);                        // Show  Page 0 dialog to current user
   }
   listen (integer chan, string name, key id, string msg)
   {
       if (msg == "<<" || msg == ">>")                   // Page change ...
       {
           if (msg == "<<")        --gPage;              // Page back
           if (msg == ">>")        ++gPage;              // Page forward
           if (gPage < 0)          gPage = gMaxPage;     // cycle around pages
           if (gPage > gMaxPage)   gPage = 0;
           BuildDialogPage(id);
           return;
       }
       if (msg != " ")                                  // no action on blank menu button
       {
           // User has selected an action from the menu
           llRegionSayTo(id, 0, "You chose action <" + msg + ">");
       }
       state ready;         // changing state will release ANY and ALL open listeners
   }
   timer()
   {
       llRegionSayTo(gUser, 0, "Too slow, menu cancelled");
       state ready;
   }
   state_exit()
   {
       llSetTimerEvent(0);          // would be dangerous to leave a dormant timer
   }

} </lsl>

Looks good. I've made one small change: I know 11 isn't supported but I think it would be better to not explode if the user is dumb enough to enable it.
Is the reason for the loop instead of llList2List to force the buttons to be strings? -- Strife (talk|contribs) 15:43, 12 December 2013 (PST)
Oh you're right! Of course I should have used llList2List(). TY. Fair enough on 11. Omei Qunhua 15:53, 12 December 2013 (PST)