[SugarCube 2.12.1] Cleanest way to handle JavaScript intervals and window properties on restart?

Rokiyo

Hello again,

I have run into a bug with my internal game loop, and while I've already come up with a couple of ways I could fix it, I'd still like some advice on the cleanest way to go about it.

For the curious, a WIP of the engine mechanics I'm working on can be seen here: http://rokiyo.net/games/cetacea/Story%20Engine.html

@TheMadExile, I would really appreciate your thoughts on the below, though I also welcome any advice anyone else has to offer too.

Issue summary
To start an internal game loop at around 30 ticks per second, I run the following code within StoryInit (because it seems to be one of the last things executed during story loading):
<<run window.tickTimer = window.setInterval(window.gametick, 33)>>

When calling UI.restart(), the restart is performed without a page load, which means the above timer is not cleared. This causes two primary issues:

• Multiple timers running at once, all of them calling window.gametick more often than expected.
• window.gametick getting called during story initialization, before specific story variables and DOM elements have started existing.

Potential solutions
I can think of several ways to address some of these issues (though none of them strike me as ideal):

• A singleton-style approach: <<run window.tickTimer = window.tickTimer || window.setInterval(window.gametick, 33)>>
• Hook into an Engine.restart() to clear the interval, possibly by rebuilding a custom UI.restart() dialog via the API to call some cleanup code if the user confirms the restart (e.g. clearInterval(window.tickTimer)).
• Wrap window.gametick in a try/catch block to silently discard any errors that occur from it running on load.
• Something involving the options parameter on UI.restart()? I don't understand the documentation within Dialog.addClickHandler() enough to know if I've missed something obvious here.

Conclusion
So yeah, there are a few ways to go about it, but all of them strike me as a little bit "hacky". I'd really appreciate a second opinion on a better approach to this.

The other thought on my mind is... If intervals aren't being cleared during Engine.restart(), does that mean all the other stuff I'm attaching to window isn't getting cleaned up either? I mean, I guess it all gets overwritten when the story JavaScript gets rerun, but I'm wondering if it would be cleaner to have some kind of shutdown task object that only gets called during a restart?

greyelf

One possible solution it to use a variable to control if the contents of the gametick function is processed or not. The following moves the initialisation of the interval to the Story Javascript area, it also moves the storage of the related code from the window object to SugarCube's setup object.

1. Story Javascript area:

/* Variable that controls if the contents of gametick is processed or not. */
setup.initialising = true;

/* The gametick function called at intervals. */
setup.gametick = function() {
	if (setup.initialising) {
		return;
	}

	/* Everything that is in your normal gametick function. */
};

/* Create a new interval only if it does not exist. */
setup.tickTimer = setup.tickTimer || window.setInterval(setup.gametick, 33);

2. StoryInit passage

/% Everything in your normal StoryInit passage. %/


<<set setup.initialising to false>>

... anytime you don't want the contents of the gametick function to be executed simply set the setup.initialising variable to true.

note: rename the variable to anything that makes sense to you.

Rokiyo

Ooh I liked that idea, but sadly it did not completely resolve the issue. It turns out that setup.gametick is being called even earlier than that, perhaps during the tear-down of the old game state.

I've narrowed the specific error down to variables() getting reinitialised almost immediately after the user clicks on restart. Enough time is passing between that and the Story Javascript getting rerun that setup.gametick is getting called while variables() is empty and while setup.initialising is still false.

That being said, I have moved all my stuff over from window to setup and started using the code to only create the interval if it doesn't already exist, so thanks for those tips.

For the time being, I'm just wrapping my gametick inside a try/catch and discarding the errors, but I'm still on the hunt for a more elegant solution than that.

TheMadExile

Rokiyo wrote: »

For the curious, a WIP of the engine mechanics I'm working on can be seen here: http://rokiyo.net/games/cetacea/Story%20Engine.html

That URL is unreachable for me as I post this—the whole domain is unreachable, actually.

Rokiyo wrote: »

Issue summary
To start an internal game loop at around 30 ticks per second, I run the following code within StoryInit (because it seems to be one of the last things executed during story loading):
<<run window.tickTimer = window.setInterval(window.gametick, 33)>>

It really doesn't matter which you use, however, since you are performing an assignment, I'd use <<set>> there, rather than <<run>>—if for no other reason than semantics.

Also, is there some particular reason you're doing that in the StoryInit special passage, rather than the Story JavaScript, which is where I'd expect to see something like that defined?

Rokiyo wrote: »

When calling UI.restart(), the restart is performed without a page load, which means the above timer is not cleared.

(emphasis mine) That is incorrect. The dialog created by UI.restart() calls Engine.restart(), which—aside from various bookkeeping—does reload the page, by calling window.location.reload().

You're not clobbering the window.location API are you?

Rokiyo

TheMadExile wrote: »

That URL is unreachable for me as I post this—the whole domain is unreachable, actually.

That server is hosted in Australia, and it seems to lose international connectivity from time to time. I'll see about getting it hosted elsewhere.

TheMadExile wrote: »

Also, is there some particular reason you're doing that in the StoryInit special passage, rather than the Story JavaScript, which is where I'd expect to see something like that defined?

Just to ensure it happens dead last, after everything else has been initialised. StoryInit appears to get processed after the Story JavaScript, and my game tick makes use of story variables via a call to variables().

TheMadExile wrote: »

You're not clobbering the window.location API are you?

No, I was just misunderstanding what I was looking at, my apologies. I did realise a page reload was occuring when I was trying out greyelf's suggestion earlier, but forgot to mention it when I responded.


Based on comments from you and greyelf, I have boiled down my root cause to this:

• I'm calling a function every 33ms
• This function calls variables() each time it runs.
• During a restart, before the window.location.reload() occurs, the story variables appear to get destroyed.
• As a result, calls such as variables().gameClock.getTime() start throwing exceptions until the reload occurs.

Is the crux of my issue just that I'm directly referencing and manipulating story variables via JavaScript? Or am I just making too big a deal out of an edge case and should just let exception handling take care of it?

TheMadExile

Rokiyo wrote: »

TheMadExile wrote: »

Also, is there some particular reason you're doing that in the StoryInit special passage, rather than the Story JavaScript, which is where I'd expect to see something like that defined?

Just to ensure it happens dead last, after everything else has been initialised. StoryInit appears to get processed after the Story JavaScript, and my game tick makes use of story variables via a call to variables().

That shouldn't be an issue if your gametick callback is properly written, which I'm going to assume is not currently the case and is the actual root of all your problems.

May we see its code?

Rokiyo

Sure, apologies for the mess: Game programming is still pretty new to me so please feel free to correct anything I've done incorrectly or inefficiently.

This just the gametick function, let me know if you want to see the code for other functions called by it.

setup.gametick = function() {
  try {
    /* calculate time since last tick */
    var now = new Date().getTime(),
        interval = now - setup.lastTick;
		
    setup.lastTick = now;

    /* grab relevant values */
    var vars = variables(),
        timeSystem = setup.timeSystem,
        gameClockTime = vars.game_clock.getTime(),
        timeGoal = timeSystem.timeGoal,
        maxTimeFactor = timeSystem.maxTimeFactor;
    
    /* calculate new time factor, used to determine how quickly time should pass */
    if (Dialog.isOpen()) {
      /* Dialog is open, so pause the clock */
      vars.time_factor = 0;
    }
    else if (timeGoal && timeGoal.getTime() > (gameClockTime-300000)) {
      /* We're in the middle of a time jump, so manage time acceleration via setting time factors */
      var newFactor,
          //grab relevant values from timeSystem
          timeGoalTime = timeSystem.timeGoal.getTime(),
          goalStartTime = timeSystem.goalStart.getTime(),
          decelWindow = timeSystem.decelWindow,
      
          //determine whether we should be speeding up or slowing down
          bHalfway = gameClockTime > ((goalStartTime+timeGoalTime) / 2),
          bDecelWindow = gameClockTime > (timeGoalTime - decelWindow);

      if (bHalfway && bDecelWindow) {
        /* Start decelerating */
        newFactor = vars.time_factor - (vars.time_factor * 0.25);
      } else {
        /* Keep accelerating */
        newFactor = vars.time_factor + (vars.time_factor * 0.25);
      }
    
      /* Clamp time factor to speed limit */
      newFactor = Math.min(Math.max(newFactor, 1), maxTimeFactor);
      vars.time_factor = newFactor;
    } else {
      /* We're back in normal time */
      timeSystem.timeGoal = undefined;
      vars.time_factor = 1;
    }
  
    timeSystem.addTime(interval);  //increments the game clock
    setup.uiSystem.updateUI(); //updates DOM
    setup.eventSystem.runEvents(); //check if any scheduled events are due to fire off
  }
  catch (e) {
    console.log("gametick failed: " + e.message);
  }
};

TheMadExile

I would probably suggest changing the top like so:

setup.gametick = function() {
  try {
    var vars = variables();

    /* no-op if $game_clock doesn't exist */
    if (!vars.game_clock) {
        return;
    }

    /* calculate time since last tick */
    var now = Date.now(),
        interval = now - setup.lastTick;
		
    setup.lastTick = now;

    /* grab relevant values */
    var timeSystem = setup.timeSystem,
        gameClockTime = vars.game_clock.getTime(),
        timeGoal = timeSystem.timeGoal,
        maxTimeFactor = timeSystem.maxTimeFactor;

The primary thing was moving the declaration of vars to the top and adding a bailout check for $game_clock. I also replaced now = new Date().getTime() with now = Date.now(), as it yields the same value without requiring the instantiation of a new Date object, which you immediately discard.

DairynGM

I checked out your game UI. It's sexy! I like it.