Updated OSE NPC timer scripts

For test results, bug reports, announcements of new builds etc.

Moderators: winston, another_commander, Getafix

pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Updated OSE NPC timer scripts

Post by pmw57 »

While updating OSE we found several NPC ship scripts that use timers, some of which inadvertantly crash the game due to the timer still going after the NPC ship has died.

All of the ship-based timer scripts are: All-in-one - ose npc timer scripts.7z

This thread is to keep track of information related to these NPC timer scripts, along with general updates to their code as well.
Last edited by pmw57 on Thu Oct 15, 2009 10:36 am, edited 13 times in total.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

The playerWillExitSystem event is purpose-designed to allow NPC ships to perform necessary tasks before the player exists the system.

This is when it is best to stop and delete any timers that the ship script may have created.

Code: Select all

this.playerWillExitSystem = function () {
	if (this.cloakingTimer) {
		this.cloakingTimer.stop();
		delete this.cloakingTimer;
	}
};
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

It seems that a lot of code is being taken up with conditions for timers.

I propose that we come up with a consistent way to manage timers, something like this:

Code: Select all

this.startTimer = function (timerName) {
	var timerSettings;
	if (this[timerName]) {
		this[timerName].start();
		return this[timerName];
	} else if (this.timerSettings[timerName]) {
		timerSettings = this.timerSettings[timerName];
		return new Timer(this, timerSettings.func, timerSettings.delay, timerSettings.interval);
	}
	return null;
};
this.stopTimer = function (timerName) {
	if (this[timerName]) {
		this[timerName].stop();
		delete this[timerName];
	}
};
To use, you would create the timer settings that are to be used:

Code: Select all

this.timerSettings = {
	'damageControlTimer': {func: this.repairShip, delay: 60, interval: 300}
};
Then we will just need to issue a one-line statement to start or stop a timer.

Code: Select all

this.shipExitedWitchspace = this.shipLaunchedFromStation = function () {
	if (player.ship.hasEquipment("EQ_DCN")) {
		this.damageControlTimer = this.startTimer('damageControlTimer');
	}
};

Code: Select all

this.playerWillExitSystem = function () {
	this.stopTimer('damageControlTimer');
	this.stopTimer'(damageNodeTimer');
};
The only issue that I have found is that this.timerSettings needs to occur after the function has been defined, so placing this code at the end of the script is a good idea.

Opinions? Insults?
Last edited by pmw57 on Thu Oct 15, 2009 6:07 am, edited 2 times in total.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
User avatar
Thargoid
Thargoid
Thargoid
Posts: 5528
Joined: Thu Jun 12, 2008 6:55 pm

Re: Updated OSE NPC timer scripts

Post by Thargoid »

pmw57 wrote:
While updating OSE we found several NPC ship scripts that use timers, and inadvertantly crash the game due to the timer still going after the NPC ship has died.

The scripts that are involved with this are:
  • behemoth-carrier.js - discuss
  • buzzer-wowbagger-insult.js - download
  • CaduceusAutorepair.js
  • fuelStation_satellite.js
  • fuelStation_station.js
  • PlayerFuelStation_station.js
This thread is to keep track of information related to these NPC timer scripts.
The current release version of Fuel station and fuel satellite scripts (and presumably PlayerFuelStation_station, although I didn't write that) already tidy up after themselves when the player leaves the system, hence should be no risk for crashes.

Please show me your proof that their script timers are crashing the game.

Code: Select all

this.playerWillEnterWitchspace = this.playerGone = function()
	{
	if(this.scanTimer)
		{
		this.scanTimer.stop();
		delete this.scanTimer;
		}
	}
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Re: Updated OSE NPC timer scripts

Post by pmw57 »

Thargoid wrote:
The current release version of Fuel station and fuel satellite scripts (and presumably PlayerFuelStation_station, although I didn't write that) already tidy up after themselves when the player leaves the system, hence should be no risk for crashes.

Please show me your proof that their script timers are crashing the game.
My apologies for tarring you with that brush. I shall narrow the scope by saying only "some of which" instead.

You're not off the hook yet though. :twisted:
The fuel station code will be brought up to meet the same set of minimum standards that are being applied to the other scripts too.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
User avatar
Thargoid
Thargoid
Thargoid
Posts: 5528
Joined: Thu Jun 12, 2008 6:55 pm

Post by Thargoid »

Just apply the engineering addage - if it isn't broken, don't fix it. Unless you want to risk starting to upset people.

But if we're doing something wrong, just tell us and let us look after our own scripts perhaps?
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

Thargoid wrote:
Just apply the engineering addage - if it isn't broken, don't fix it. Unless you want to risk starting to upset people.
I'm coming not from an engineering background, but a programming background. Code needs to change. Why? How much code is good enough to stay around for decades without change? Virtually none of it. Code that doesn't improve becomes buggy, and broken.

Additionally, the code here is not in a vacuum. It is open for view by everyone who downloads it. A bit of spit and polish does not go astray.

Edit: What I'm doing here is to ensure that the scripts not only appear to work in the game, but are PROVEN to work without issues. It's the equivelent using official "building standards" in construction work.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

@Thargoid - don't panic.

I've just gone over the fuel station code (satellite & station) and there is nothing to worry about. I know that it's stressful having code put under the spotlight, but it really is for the best.

The only changes have been to bring the code presentation in line with widely regarded JavaScript notation standards. Some statements didn't end with a semicolon, some conditional blocks didn't have their code block denoted with braces, small-fry stuff.

Like with the other scripts, the code now meets the minimum standards that are set for good and maintainable JavaScript code.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
User avatar
Thargoid
Thargoid
Thargoid
Posts: 5528
Joined: Thu Jun 12, 2008 6:55 pm

Post by Thargoid »

Even with the best and most helpful will in the world, you're coming across via the cold medium of the internet as rather arrogant at the moment. It doesn't exactly inspire a desire to collaborate.

I don't doubt that certainly my code could sometimes do with "a bit of spit and polish" on occasions, but as I said currently it seems to work within the bounds of Oolite's engines. Yes we have had some examples of code causing crashes, at which point they are hunted down, the code debugged and we carry on as a team.

At the moment all of this is rapidly escalating the option of removing support and goodwill towards the OSE project, at least from me.
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

I find myself faced with a dilemma. While I have taken upon my shoulders the responsibility of being in charge of OSE scripting code, how am I to ensure that the code meets a certain minimum standard of quality?

Am I to point people to the JavaScript beautifier or even better to JS Lint with careful instructions on how to massage it to work best with script code in the Oolite environment?

Or perhaps point them to some of the better JavaScript resources that actually explain in a way that makes sense why things are done a certain way, and the troubles that they prevent?

Even after doing all of that there's no guarantee that people will actually take these resources and apply it to their own code, because we all know that our own code is best understood by us as it is now, not after someone has had their elbows in their mucking about doing god-knows what.

Perhaps you might have some advice for me, on how to bring potentially dodgy code up to scratch while maintaining a minimum set of standards that all code should follow.

It can be done easily and effectively as shown here, but to do so without ruffling feathers? JavaScript is my strong suit, the other skills for dealing with people I'm not so strong at.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

By the way, do you think that playerWillExitSystem should be used instead of playerWillEnterWitchspace, as advised by Commander McLane?
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Post by Eric Walch »

What I miss in the behemoth script is the way I do deal with timers in the 2.5.2 release. That method was already tested against timers before the 1.73 release. Kaks improved that code even a bit and put a good solution on the wiki page about timers, somewhere before the 1.73 release.
I didn't update the behemoth script with my latest work until this week, but I would have guessed that when others were working on timer problems, they would at least consult the wiki page and I would find part of the solution in the now presented code.
pmw57 wrote:
By the way, do you think that playerWillExitSystem should be used instead of playerWillEnterWitchspace, as advised by Commander McLane?
No, look at the wiki page about handlers.
Last edited by Eric Walch on Thu Oct 15, 2009 8:16 am, edited 1 time in total.
User avatar
Lestradae
---- E L I T E ----
---- E L I T E ----
Posts: 3095
Joined: Tue Apr 17, 2007 10:30 pm
Location: Vienna, Austria

...

Post by Lestradae »

Thargoid wrote:
... you're coming across via the cold medium of the internet as rather arrogant at the moment. It doesn't exactly inspire a desire to collaborate. ... At the moment all of this is rapidly escalating the option of removing support and goodwill towards the OSE project, at least from me.
You know T, I am a bit surprised. Please read your above statements after counting to ten, and then perhaps once or twice again after counting to ten. How do they feel?

You are also coming across the cold medium of the internet :wink:

There was a lot of "rapid escalation" towards my project in the past. After many hundred work hours of working on it and also having to defend it here over and over again - while it has proven to be one of the most successful oxp ideas measured at the interest it generates in players - I'm not sure who is coming across as arrogant here.

I really don't want to step on your toes here (and that's not easy with OSE - I am aware that it does step on toes) - but here is someone new on the board who is ready to look through everything and find problems, report them and even fix them - where is the problem with that one? He's trying to help, for frak's sake! :)

Someone who is new here gets bombarded with repeated preachings to read the wiki, FAQs, use the search function on this board, and is sometimes put down due to stumpling first attempts. And rightly so! It ensures that people try their best - there's no problem with that, is there?

But if someone experienced and able as you might get advice, it's arrogant?

Please, let's talk this out not in the usual RS/OSE way, as I'm tired of that and won't do it. Don't threaten to "walk out" if something bothers you, talk it out in the open and be done with it so that we can get back to work.

OK? Or not? I'd really find it sad if we got into a rather pointless ruffle about this one, T :wink:

Shanti 8)

L

PS: Perhaps we all have to get our heads around collaborating on something like that. Except the devs who are probably used to it. Good will simply is nescessary to find a way to do so. Methinks.
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

Eric Walch wrote:
pmw57 wrote:
By the way, do you think that playerWillExitSystem should be used instead of playerWillEnterWitchspace, as advised by Commander McLane?
No, look at the wiki page about handlers.
The wiki page about handlers seems to be light on the topic.

It has a brief mention about how tickle handler might be necessary until the timer mechanism is implemented, but nothing more about timers as such.

I'm having trouble finding Kaks related wiki update that relates to our issue here.
http://wiki.alioth.net/index.php/Specia ... tions/Kaks

Until you get back to us about this, I'll investigate your most recent behemoth script to try and puzzle things out.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
User avatar
Kaks
Quite Grand Sub-Admiral
Quite Grand Sub-Admiral
Posts: 3009
Joined: Mon Jan 21, 2008 11:41 pm
Location: The Big Smoke

Post by Kaks »

20:30, 25 August 2009 (hist) (diff) Oolite JavaScript Reference: Timer
20:13, 25 August 2009 (hist) (diff) Oolite JavaScript Reference: Timer

Are both on that list, and seem to refer to timers. It might be what Eric is referring to.

Regards.
Hey, free OXPs: farsun v1.05 & tty v0.5! :0)
Post Reply