nickgammon / mushclient

Open Source Windows MUD game client
www.mushclient.com
182 stars 76 forks source link

Here is a little bug in trigger process #43

Closed suxiaojack closed 3 years ago

suxiaojack commented 6 years ago

If a trigger's sendto text is simple script sentences,the process is working right.

But if used complex 'WAIT.REGEXP' function in script text and be on KeepEvaluating ,the trigger would be fired endless in looping.

I think here is some bug in CMUSHclientDoc::ProcessPreviousLine , because the WAIT.REGEXP function dynamically created some new triggers,and the SortTriggers function be called , this would be broken .

// main triggers
    for (iItem = 0; !bFoundIt && iItem < GetTriggerArray ().GetSize (); iItem++)

void  CMUSHclientDoc::SortTriggers (void)
  {

int iCount = GetTriggerMap ().GetCount ();
int i;
CString strTriggerName;
CTrigger * pTrigger;
POSITION pos;

  GetTriggerArray ().SetSize (iCount);
  GetTriggerRevMap ().clear ();

  // extract pointers into a simple array
  for (i = 0, pos = GetTriggerMap ().GetStartPosition(); pos; i++)
    {
     GetTriggerMap ().GetNextAssoc (pos, strTriggerName, pTrigger);
     GetTriggerArray ().SetAt (i, pTrigger);
     GetTriggerRevMap () [pTrigger] = strTriggerName;
    }

  // sort the array
  qsort (GetTriggerArray ().GetData (), 
         iCount,
         sizeof (CTrigger *),
         CompareTrigger);

  } // end of CMUSHclientDoc::SortTriggers

Maybe the iItem++ point to same trigger repeatly?

nickgammon commented 6 years ago

I don't see how that would cause a problem because of the rest of the code in the loop:

    for (iItem = 0; !bFoundIt && iItem < GetTriggerArray ().GetSize (); iItem++)
      if (GetTriggerArray () [iItem] == trigger_item)
        {
        bFoundIt = true;
        // execute trigger script
        ExecuteTriggerScript (trigger_item, strCurrentLine, StyledLine);
        }

Before the script is executed bFoundIt is set to true, and thus the loop exits, regardless of whether items are added to the array or not.

Can you please post code to reproduce this problem?


Sorry for the delay in replying, I was on holidays.

suxiaojack commented 6 years ago

Hi, nickgammon! I'm sorry, I can not accurately locate the position of the problem. However, I use program in Chinese. A regular matching trigger does have the problem of being triggered repeatedly (the times is uncertain, one, two, three, and so on are possible, sometimes leading to an infinite loop). So,I must wrap my wait.make function code in these format:

if lock_mark~=true then
        lock_mark=true
        wait.make(function()
              ...
             lock_mark = nil
       end)
    end

The above code form ensures that an infinite loop does not occur, but does not prevent the trigger from being repeatedly triggered occasionally.

I thought,

// main triggers
    for (iItem = 0; !bFoundIt && iItem < GetTriggerArray ().GetSize (); iItem++)

The item pointer would be broken,by new trigger created and the qsort function be called.

Any Trigger match text,but KeepEvaluating,suppose it is 'hello':

Bad:

require "wait"
wait.make(function()
    Execute("say some_other_wait")
    local l,w=wait.regexp("some_other_wait",1)
    --long function 
    if l~=nil then
      wait.make(function()
          Execute("say I would fail")
      end)
    end
end)

Good:

require "wait"
if hello_mark~=true then
   hello_mark=true
wait.make(function()
    Execute("say some_other_wait")
    local l,w=wait.regexp("some_other_wait",1)
    --long function 
    if l~=nil then
      wait.make(function()
          Execute("say I would fail")
      end)
    end
end)
end

Simulate('hello\n') to test.
nickgammon commented 5 years ago

I don't see why that should fail. I tested with this:

require "wait"
wait.make(function()
    Execute("say some_other_wait")
    local l,w=wait.regexp("some_other_wait",1)
    print "done"
end)

Even after repeatedly simulating "hello" it worked without going into a loop.

nickgammon commented 3 years ago

Closed due to lack of activity.