----------

Started by ------, Thu 08/08/2013 00:18:07

Previous topic - Next topic

------

---------

Gilbert

So, what's your problem? The expired action not executing, or the action carried out earlier than expected?
From my quick glimpse of your codes I guess it's the latter case.
Note that timers measure time passed in game cycles, not seconds.
So, unless you have set up your game to run as slow as 1 FPS, the following line:
Code: AGS
SetTimer(4, 4);

will make the timer expire almost immediately, which under the default setting of 40 FPS, will end in 0.1 second.
To make it expire in say, 4 seconds under default settings, you have to use:
Code: AGS
SetTimer(4, 160); //40 * 4 = 160


And before some unfriendly people may like to point this out, it's recommended to properly format the attached codes, with the [code=AGS][/code] tags (or click the '#' button in the post editor) other than manually just italicising them.

monkey0506

I'll second everything Gilbot said. When asking for help, definitely post the relevant code, but also please give more details as to what is actually wrong (e.g., what is the behavior you expect vs. what is the behavior you get?).

As to the issue of game loops, I generally advise against just statically using "40" as the loops per second because there is the GetGameSpeed function which returns the current speed. Most people don't ever actually change the game speed from the default 40, but this covers you in the event you do, and preserves your timing for these sequences:

Code: ags
SetTimer(4, GetGameSpeed() * 4); // 4 seconds, regardless of actual game speed


Of course I also think SetTimer should be deprecated entirely, but that's another issue...

Crimson Wizard

Quote from: monkey_05_06 on Thu 08/08/2013 18:07:17
Of course I also think SetTimer should be deprecated entirely, but that's another issue...
Hey, monkey, you know, it is very impolite to make statements like that and not provide any further explanation.

monkey0506

#4
Very well.

SetTimer/IsTimerExpired is based on a useful concept but the fundamental flaw in its design comes from the sheer fact that it uses arbitrary integer IDs for the timers. In a large scale project, it could easily become unwieldy to keep track of which timer is being used for what particular event, especially if you have multiple timers running simultaneously. This problem is only worsened if a module author is ignorant enough to use SetTimer in their module.

One of the most basic concepts in programming that you could hope to learn is scope. Most of your timers likely do not need global scope (obviously this depends on game design, but I am speaking very generally here). By "global scope" I am referring to the fact that SetTimer affects a set of game-wide timers. Every script shares access to the same set of timers. In all likelihood, any timed events will probably be carried out by the room script (unless for example, it occurs when interacting with a character or inventory item, and so forth). Unless there is a specific reason that you need to check the state of the timer outside of that room, then for that particular timed event the broadest scope you need is room scope. If it's a blocking event you could even handle it at function scope.

In programming, best practices state that you should generally work to define things in the narrowest scope in which they need to be used (unless you have a good reason not to). Using timers at global scope when you likely only need room or function scope breaks these best practices.

So between the usage of ambiguous and arbitrary integer IDs and the global scope, SetTimer is a broken function which leads to bad programming.

A better way of handling the situation would be to use simple named integer variables at an appropriate scope. Not only does this prevent the ambiguity of using an ID, it allows you to specify exactly the scope you need and gives you greater control over when your timer is updated. Using your code as an example:

Code: ags
int libroReloj = -1; // I don't speak Spanish, but I assume that's close enough

function oRegistro_Interact()
{
  if (libro == 0)
  {
    player.Walk(500, 200, eBlock);
    player.Say("Quizás podría mirar en el registro...");
    oRegistro.Visible = false;
    oRegistro2.Visible = true;
    libroReloj = (4 * GetGameSpeed());
    player.Say("Con la recepcionista aquí va a ser imposible mirarlo.");
    libro = 2;
  }
}

function room_RepExec()
{
  if (libroReloj == 0) // when the timer reaches 0
  {
    libro = 0;
    oRegistro2.Visible = false;
    oRegistro.Visible = true;
  }
  if (libroReloj >= 0) libroReloj--; // update the timer ( >= 0, so -1 will turn it off)
}


This does add the burden that you are responsible for defining and updating the timer yourself, but as I said, that also gives you greater control over when the timer gets updated (for example, if you wanted a timer to run only while the mouse was over a certain hotspot).

This fulfills the requirements for a proper timer. If you needed to check the timer from multiple scripts, then I would suggest creating it as a Global Variable (in the relevant pane) and updating in the GlobalScript. As I said, if it were a blocking method you could also create a variable inside the function itself to keep track of iterations instead of using a broader scope timer variable.

I wasn't trying to be impolite by not elaborating on this before, but there you have it.

If a global timer mechanism were to be offered by the engine (as a replacement to the broken SetTimer), it should operate on a user-defined name (as a String parameter) rather than an arbitrary integer ID. And the equivalent IsTimerExpired method should abort if given an undefined name, because it would likely be indicative of a typo.

Snarky

Quote from: monkey_05_06 on Fri 09/08/2013 10:19:05
If a global timer mechanism were to be offered by the engine (as a replacement to the broken SetTimer), it should operate on a user-defined name (as a String parameter) rather than an arbitrary integer ID. And the equivalent IsTimerExpired method should abort if given an undefined name, because it would likely be indicative of a typo.

Wouldn't it be better to make Timers proper managed objects, so you could write code like:

Code: AGS

Timer* tBookOpen = new Timer();

function oRegistro_Interact()
{
  // ...
  tBookOpen.Set(4, eSeconds); // With eGameCycles as the other, default parameter option
  // ...
}

function room_RepExec()
{
  if(tBookOpen.IsExpired())
  {
    // ...
  }
}


... or something like that? (I'd recommend thinking through the API a little better before going ahead with this change)

Crimson Wizard

Uhh, I only wished monkey_05_06 explained his idea on how one should write a script without using SetTimer, and why. ;)

Gilbert

Awww. No more OO crap please, at least make everything possible to be referred by numbers at the same time. The new audio system is already horribly broken.

monkey0506

#8
Actually, I like Snarky's suggestion, and it does keep with the way most other things are implemented...despite Gilbot's utter loathing of OO. Although, all this really has nothing to do with the thread at hand, which is why I was going to just drop the issue in the first place. (laugh)

Edit: I liked Snarky's idea so much, I decided to steal it. The implementation was what really hooked me though, because it looks and feels like proper OO despite internally just accessing a global (file scope) cache. 8-) The ultimate upshot of doing it this way is that the end user doesn't have to worry about updating the timer manually (it automatically updates), though they can still have that level of precise control as needed (e.g., pausing a timer, disallowing it to run during blocking events, etc.).

------

#9
--------

SMF spam blocked by CleanTalk