RepExec sequence not working. (Simple traffic light)

[KodiakBehr]

I've been at this for far too long and I'm likely missing something obvious.  A fresh set of eyes would be much appreciated.

Coding a traffic light at a 4-way intersection (EW and NS).  The lights cycle as all traffic lights do.  Not concerned with the interval between the lights, that's easily altered.  Just want to see the transitions work.

I have unique traffic light sprites in 133 to 137.
I have a single object, otrafficlights1, that is currently set to Graphic 133.


I'm using the following code in the room:

Code: ags

function room_RepExec()
{
//starts with EW - Green and NS - Red
SetTimer(1, 20);

if (IsTimerExpired(1)){
  otrafficlights1.Graphic=134; // EW - Yellow, NS - Red
  SetTimer(2, 20);
}
  else if (IsTimerExpired(2)){
  otrafficlights1.Graphic=135; // EW - Red, NS - Red
  SetTimer(3, 20);
  }  
  else if (IsTimerExpired(3)){
  otrafficlights1.Graphic=136; // EW - Red, NS - Green
  SetTimer(4, 20);
  }  
  else if (IsTimerExpired(4)){
  otrafficlights1.Graphic=137; // EW - Red, NS - Yellow
  SetTimer(5, 20);
  }  
  else if (IsTimerExpired(5)){
  otrafficlights1.Graphic=135; // EW - Red, NS - Red
  SetTimer(6, 20);
  }  
  else if (IsTimerExpired(6)){
  otrafficlights1.Graphic=133; // Back to EW - Green, NS - Red
  SetTimer(1, 20);
  }  

Any ideas why the lights aren't changing at all?

Thanks, in advance,

KB

[Snarky]

You're calling SetTimer from within RepExec, so it will be reset every cycle, and never time out.

[KodiakBehr]

Ha ha ha!  Thank you.  That was silly.

[Daniel Eakins]

Alternatively you can also set a view to the object and animate it the traditional way, if you need to save those timers for other purposes.

[monkey0506]

Personally I think that the timer functions should be entirely deprecated. It promotes really bad scripting form (like using a half a dozen timers where only one or two variables would actually be needed).

[KodiakBehr]

In this case, however, I am scripting cars and pedestrians to pass through the intersection when the lights are green, and stop when the lights are red. 

With that in mind, would timers not be the most appropriate mechanism?

[Snarky]

Well, you certainly don't need 6 different timers, since only one of them is running at a time. Just have one timer and one variable to keep track of the current state, and when the timer times out, use the state variable to decide which bit of code to run.

[monkey0506]

Sure, until you forget that you're using timer 6 in the room script of room 12 and end up setting timer 6 in the global script and then spend 30+ hours debugging the issue.

Code: ags

// room script
enum TrafficLightState
{
  eTrafficLightEWGreenNSRed = 133,
  eTrafficLightEWYellowNSRed = 134,
  eTrafficLightEWRedNSRed = 135,
  eTrafficLightEWRedNSGreen = 136,
  eTrafficLightEWRedNSYellow = 137,
  eTrafficLightInvalid = 138
};

TrafficLightState state;
int delay = 20;

function room_RepExec
{
  delay--;
  if (!delay)
  {
    state++;
    delay = 20;
    if (state == eTrafficLightInvalid)
    {
      otrafficlights1.Graphic = eTrafficLightEWRedNSRed;
      state = eTrafficLightEWGreenNSRed - 1; // AGS allows invalid values in an enum, so this works
    }
    else otrafficlights1.Graphic = state;
  }
}

Snarky posted while I was typing this out.

The enum was completely unnecessary, but it helps made the code more readable (IMO) versus the actual sprite numbers.

[Daniel Eakins]

Or alternatively:

Code: ags

// room script file

function room_Load()
{
  otrafficlights1.SetView(ViewWithTheRelevantGraphicsInALoop);
  otrafficlights1.Animate(0, 20, eRepeat, eNoBlock);
}

And then, to move the cars and pedestrians:

Code: ags

function room_RepExec()
{
  if (otrafficlights1.Graphic == 133) {  ...  }
  if (otrafficlights1.Graphic == 134) {  ...  }
  ...
}

(With or without the enum)

[KodiakBehr]

Crap.  That's much more elegant.  Makes me wonder how many other things I've coded in a needlessly sloppy fashion.  Oh well, all part of learning, I guess.