The Oolite Extended Project - Fork, no oxp

[pmw57]

Hi pmw57,

the new scripts give me quite some of these here, multiple of them:

[script.javaScript.warning.undefinedProp]: ----- JavaScript warning ("PlanetFall" 1.2): reference to undefined property this.rangeCheckTimer
[script.javaScript.warning.undefinedProp]: ../AddOns/A - OSE Main Data WiP V0.70.14.oxp/Scripts/PlanetFall.js, line 496.
[script.javaScript.exception.noProperties]: ***** JavaScript exception ("PlanetFall" 1.2): TypeError: this.rangeCheckTimer has no properties
[script.javaScript.exception.noProperties]: ../AddOns/A - OSE Main Data WiP V0.70.14.oxp/Scripts/PlanetFall.js, line 496.

Other than that, the V0.7 with that (pmw57's) script patch has reduced the crashes massively. Had one, while docked at the main station and saving

/report

L

Thanks, that looks to be legacy code at work. Here is the code that is related to rangeCheckTimer. Unrelated code has been stripped out, for the sake of simplification.

Code: Select all

this.shipExitedWitchspace = function () {
	this.rangeCheckTimer.start();
};
this.shipLaunchedFromStation = function () {
	if (this.rangeCheckTimer) {
		this.rangeCheckTimer.start();
	} else {
		this.rangeCheckTimer = new Timer(this, this.planetScan, 0, 15);
	}
};
this.shipExitedPlanetaryVicinity = function () {
	this.planetScan();
	this.rangeCheckTimer.start();
};
this.shipWillDockWithStation = function (station) {
	this.rangeCheckTimer.stop();         // Line 496
};
// My added code is here
this.playerWillEnterWitchspace = function () {
	if (this.landingTimer) {
		this.landingTimer.stop();
		delete this.landingTimer;
	}
	if (this.rangeCheckTimer) {
		this.rangeCheckTimer.stop();
		delete this.rangeCheckTimer;
	}
};

Does anyone else see the problem? How about when you hyperspace in to a system and then dock - a timer no longer exists.

I reckon that the following needs to be added to the shipExitedWitchspace event, so that if a timer doesn't exist, one is created at that time.

Code: Select all

	if (this.rangeCheckTimer) {
		this.rangeCheckTimer.start();
	} else {
		this.rangeCheckTimer = new Timer(this, this.planetScan, 0, 15);
	}

[pmw57]

pmw57, can I suggest you and Screet have a talk together concerning the script updates? Because I am now getting them from both of you, and I guess four eyes see more than two eyes, and I am useless at checking the scripts except finding the most blatant errors or when gametesting.

If I can get the updated script from either you or Screet (you'll win a free drink for your effort) I'll compare, constract, and update the existing script that I have here, which will make an appearance when I have finished going through the other scripts currently involved with the timers.

After these I will go through the others, but a bit at a time.

What I'm doing is massaging the existing code so that jslint.com has little to no issues with the code. The linter warns "Warning! JSLint will hurt your feelings." and it's true, when used properly it rips you a new one. However, the code does end up being easier to read and more reliable for ongoing maintenance, which is one of the benefits of coding to a set of standards.

So please, get the updated Caduceus script through to me and I'll take you out for a drink when you're next in the area.

[Screet]

If I can get the updated script from either you or Screet (you'll win a free drink for your effort) I'll compare, constract, and update the existing script that I have here, which will make an appearance when I have finished going through the other scripts currently involved with the timers.

Just need your email, that's why I asked you via PM...and yes, the deleted timer which is then attempted to be used after witchspace/docking I've also fixed

Anyway...I had the "a simple additional check won't hurt, even if it's not necessary" approach. Makes things also safer should these methods at some point be called without a timer activating them. Could otherwise easily become a nasty bughunt after a little change that's not expected to cause trouble

Screet

[pmw57]

If I can get the updated script from either you or Screet (you'll win a free drink for your effort) I'll compare, constract, and update the existing script that I have here, which will make an appearance when I have finished going through the other scripts currently involved with the timers.

Just need your email, that's why I asked you via PM...and yes, the deleted timer which is then attempted to be used after witchspace/docking I've also fixed

Wha? That's funny, no notification occurred about the PM. Thanks for the update, cheers.

[Screet]

Wha? That's funny, no notification occurred about the PM. Thanks for the update, cheers.

Could be some problem with the board. Today I missed several email notifications about new messages on the board (not PMs)...and I've also got the debug errors quite often.

Screet

[matthewfarmery]

yeah the board been giving me issues as well

an update on the crashing issue,

Screet through email has sent me his package of scripts, so far, I haven't had any crashes, so for the time being version 0.7 is stable, I also removed the emergency energy unit, as he said it was ran by a timer, I will re add it at some point and see if the crashes come back with it or not, but anyway happy days are here again

[pmw57]

yeah the board been giving me issues as well

an update on the crashing issue,

Screet through email has sent me his package of scripts, so far, I haven't had any crashes, so for the time being version 0.7 is stable, I also removed the emergency energy unit, as he said it was ran by a timer, I will re add it at some point and see if the crashes come back with it or not, but anyway happy days are here again

When testing, has anyone ensured that the data execution prevention is NOT set for Oolite, because only when it is not set will we be able to properly test the stability of the code.

If you did set DEP and Oolite seems stable, you may be judging on false pretenses.

[matthewfarmery]

I have disabled DEP for Oolite, but will turn it back on, I never really had any issues with dep since I moved on to this CPU, (Intel q6600) as it supports hardware DEP, on my previous computers, I had issues with DEP, as it was software version, not hardware, and I had many issues and crashes, software DEP is a pain at times, I think hardware DEP is better, but anyway I will turn on DEP as before the warning in this thread, I never touched DEP settings, and in general not had any real issues, with DEP itself, yes I did have crashes with the previous build, but updating the scripts seem to have fixed that, anyway, may turn on DEP for Oolite tomorrow too tired now, off to bed in a bit

night all

[Screet]

When testing, has anyone ensured that the data execution prevention is NOT set for Oolite, because only when it is not set will we be able to properly test the stability of the code.

If you did set DEP and Oolite seems stable, you may be judging on false pretenses.

The other way around: With DEP off OOlite may appear to be stable while it isn't. The thing that DEP catches (and the reason that we have to disable it) is something that is really wrong. With DEP on it causes Oolite to stop, while Oolite does run without it. However, at least for the "timer crash test oxp" the thing that DEP catches is harmless. We need to keep in mind that there's simply no guarantee that similar situations are also harmless.

Screet

[pmw57]

Anyway...I had the "a simple additional check won't hurt, even if it's not necessary" approach. Makes things also safer should these methods at some point be called without a timer activating them. Could otherwise easily become a nasty bughunt after a little change that's not expected to cause trouble

Thanks, I'll get those worked in to the other work that's going on here.

For example, behemoth-carrier.js before restructuring

Code: Select all

this.statusCheck = function () {
	if (!this.cloakedTarget) {
		if (this.cloakingTimer) {
			this.cloakingTimer.stop();
			delete this.cloakingTimer;
		}
	} else {
		if (!this.cloakedTarget.isCloaked) {
			this.ship.target = this.cloakedTarget;
			this.ship.reactToAIMessage("TARGET_DECLOAKED");
			if (this.cloakingTimer) {
				this.cloakingTimer.stop();
				delete this.cloakingTimer;
			}
		}
	}
};

and after restructuring

Code: Select all

this.statusCheck = function () {
	if (this.cloakedTarget && !this.cloakedTarget.isCloaked) {
		this.ship.target = this.cloakedTarget;
		this.ship.reactToAIMessage("TARGET_DECLOAKED");
	}
	if (this.cloakedTarget && this.cloakedTarget.isCloaked) {
		// do nothing, keep the timer going
	} else {
		if (this.cloakingTimer) {
			this.cloakingTimer.stop();
			delete this.cloakingTimer;
		}
	}
};

[pmw57]

Can anyone confirm whether correcting planetlist to this.planetlist will have much of an impact on frame_fuel_collector.js ?

Before:

Code: Select all

			cr = this.moonlist[i].position.distanceTo(planetlist[0]);
			for (al = 0; al < this.noofplanets; al++) {
				if (cr > this.moonlist[i].position.distanceTo(planetlist[al])) {
					cr = this.moonlist[i].position.distanceTo(planetlist[al]);
					this.planetlist[al].moons = 0;
					motherplanet = al;
				}
			}

After:

Code: Select all

			cr = this.moonlist[i].position.distanceTo(this.planetlist[0]);
			for (al = 0; al < this.noofplanets; al++) {
				if (cr > this.moonlist[i].position.distanceTo(this.planetlist[al])) {
					cr = this.moonlist[i].position.distanceTo(this.planetlist[al]);
					this.planetlist[al].moons = 0;
					motherplanet = al;
				}
			}

There are also the occasional use of just moonlist instead of this.moonlist - not very useful that.

Edit: Oh come on!

Code: Select all

                this.derelictcount == this.hacktime+1;

[pmw57]

As it has had a lot of work done to it, here is the frame_fuel_collector.js script.
http://www.box.net/shared/myprzskb5z

It will be included with the other scripts files that have had timer updates, which I am currently tidying up.

After those, I will move on to tidying up the other script files as well.

[pmw57]

The OreProcessor.js seems to have some holes large enough to drive a truck through.

Code: Select all

this.canAwardCargo = function (type, quantity) {
    if (0 < oolite.compareVersion("1.74")) {
        return player.ship.canAwardCargo(type, quantity);
    } else {
        var cargo = type.toLowerCase();
        if (type = "gold" && Manifest["gold"] < 1E3-quantity) {
			return true;
		}
        if (type = "platinum" && Manifest["platinum"] < 1E3-quantity) {
			return true;
		}
        if (type = "gem-stones" && Manifest["gem-stones"] < 1E6-quantity) {
			return true;
		}
        return player.ship.cargoSpaceAvailable > quantity;
    }
};

Am correcting to type === "gold" and using this.manifest instead, and of course there's the whole type/cargo thing, and will see where we go from there.

[Lestradae]

@Screet, pmw57 & matthewfarmery:

OK, let's use the following-to-download Scripts folder as the new template on which to work on?!:

http://www.box.net/shared/static/h1fi9cco4s.zip

... so that every further evolution of the OSE scripts folder will have this one as its ancestor, in a manner of speaking!

These scripts are the worked-over ones by pmw57, with additional security/timer-madness alterations by Screet and I have looked them over so that things like the Combat Computers messages are synchronised with each other "stylistically" (<- Is that even a proper word?).

Again, to avoid double or triple work, please everyone derive all and any future scripts you upgraded or fixed from the above download link!!!

Cheers & drinks all around

L

[pmw57]

OK, let's use the following-to-download Scripts folder as the new template on which to work on?!:

2 days wasted. !@%$#%@۞