Strict mode warnings

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

Moderators: another_commander, winston, Getafix

Post Reply
User avatar
Milo
Deadly
Deadly
Posts: 236
Joined: Mon Sep 17, 2018 5:01 pm

Strict mode warnings

Post by Milo »

Code: Select all

Warning (strict mode): reference to undefined property ship.missiles[i].primaryRole
    Active script: oolite-populator 1.89
    oolite-populator.js, line 2306:
    			if (ship.missiles[i].primaryRole == "EQ_MISSILE" && chance < Math.random())
I haven't looked into this one.

Code: Select all

Warning (strict mode): reference to undefined property market[commodity].marketMaskPrice
    Active script: Oolite Trader AI 1.89
    oolite-priorityai.js, line 2543:
    				var adjust = market[commodity].marketEcoAdjustPrice * multiplier * quantity / market[commodity].marketMaskPrice;

Warning (strict mode): reference to undefined property market[commodity].marketEcoAdjustPrice
    Active script: Oolite Trader AI 1.89
    oolite-priorityai.js, line 2543:
    				var adjust = market[commodity].marketEcoAdjustPrice * multiplier * quantity / market[commodity].marketMaskPrice;
For the market warnings, I at first thought of an unknown commodity name (which would also be a problem here), but when I scanned in-system ships I found no commodities in their cargoList that didn't have an entry at the main station. However, none of the system.mainStation.market entries had a marketEcoAdjustPrice or marketMaskPrice property.

I think it could be fixed by enclosing lines 2543 and 2544 in an if block like this:

Code: Select all

				if (market[commodity].marketEcoAdjustPrice && market[commodity].marketMaskPrice) // defined and non-zero (for division)
				{
					var adjust = market[commodity].marketEcoAdjustPrice * multiplier * quantity / market[commodity].marketMaskPrice;
					profit += adjust;
				}
Next warning:

Code: Select all

Warning (strict mode): reference to undefined property this.__ltcache.oolite_homeStation
    Active script: YAH patrol AI 4.5.1
    oolite-priorityai.js, line 903:
    	if (this.__ltcache.oolite_homeStation === null)
Suggested revision beginning at line 901 of oolite-priorityai.js:

Code: Select all

PriorityAIController.prototype.homeStation = function() 
{
	if (this.__ltcache.oolite_homeStation !== undefined)
	{
		if (this.__ltcache.oolite_homeStation === null)
		{
			return null;
		}
		if (this.__ltcache.oolite_homeStation.isValid)
		{
			return this.__ltcache.oolite_homeStation;
		}
	}
	// home station has not been assigned yet (property undefined)
	// home station might be the owner of the ship, or might just
	// be a group member
	if (this.ship.owner && this.ship.owner.isStation && this.friendlyStation(this.ship.owner))
	{
		this.__ltcache.oolite_homeStation = this.ship.owner;
		return this.ship.owner;
	}
	if (this.ship.group)
	{
		var gs = this.ship.group.ships;
		for (var i = gs.length-1 ; i >= 0  ; i--)
		{
			if (gs[i] != this.ship && gs[i].isStation && this.friendlyStation(gs[i]))
			{
				this.__ltcache.oolite_homeStation = gs[i];
				return gs[i];
			}
		}
	}
	this.__ltcache.oolite_homeStation = null;
	return null;
}
Next pair of warnings (note, my line numbers may be slightly different due to changes mentioned above):

Code: Select all

Warning (strict mode): reference to undefined property this.ship.group.leader.AIScript.oolite_intership.cargodemanded
    Active script: Oolite Pirate AI 1.89
    oolite-priorityai.js, line 2039:
    			if (this.ship.group.leader && this.ship.group.leader.AIScript.oolite_intership && this.ship.group.leader.AIScript.oolite_intership.cargodemanded > 0)

Warning (strict mode): reference to undefined property this.ship.group.ships[0].AIScript.oolite_intership.cargodemanded
    Active script: Oolite Pirate AI 1.89
    oolite-priorityai.js, line 2048:
    			else if (this.ship.group.ships[0].AIScript.oolite_intership && this.ship.group.ships[0].AIScript.oolite_intership.cargodemanded > 0)
One way to "fix" this (the code works, but strict mode dislikes it) without too much added code is to provide a default value for the comparison, like this:

if (this.ship.group.leader && this.ship.group.leader.AIScript.oolite_intership && (this.ship.group.leader.AIScript.oolite_intership.cargodemanded || 0) > 0)

With this, if this.ship.group.leader.AIScript.oolite_intership.cargodemanded is undefined, the || 0 replaces it with zero. The result of 0 > 0 is false. undefined > 0 also is false, but also triggers a warning.

User avatar
Milo
Deadly
Deadly
Posts: 236
Joined: Mon Sep 17, 2018 5:01 pm

Re: Strict mode warnings

Post by Milo »

Another one (line number may be slightly off due to my local edits):

Code: Select all

Exception: TypeError: group.leader.group is null
    Active script: Oolite Escort AI 1.89
    oolite-priorityai.js, line 1105:
    			gs = group.leader.group.ships;

cag
Dangerous
Dangerous
Posts: 98
Joined: Fri Mar 17, 2017 1:49 am

Re: Strict mode warnings

Post by cag »

cag wrote:
Thu Oct 05, 2017 2:52 am
...
<long rambling post>
...
TL;DR I fell down a deep rabbit hole and applied strict mode and various optimizations to all the scripts in the Resources\Scripts folder that comes with the core. I left the AI scripts alone, as I would need more I before messing with A's :)

So before you fall into that same rabbit hole, try substituting these and see if the problems persists. The core scripts have changed little in the past (almost 3 f@#! years) and I've kept them current. I've been using them since then but I don't have your knack of blowing up code!

https://www.dropbox.com/s/cudwuq8d56epc ... s.zip?dl=0
"Better to be thought a fool, boy, than to open your trap and remove all doubt." - Grandma [over time, just "Shut your trap... fool"]
"The only stupid questions are the ones you fail to ask." - Dad
How do I...? Nevermind.

User avatar
Milo
Deadly
Deadly
Posts: 236
Joined: Mon Sep 17, 2018 5:01 pm

Re: Strict mode warnings

Post by Milo »

Thanks. I'm confused, though, why move "use strict"; inside the scope of the this.activated function in oolite-cloaking-device-equipment.js and why name the function with 'this.activated = function activated()' instead of just 'this.activated = function()'? In the core version of the file, it has "use strict"; at the top outside the function and omits naming the function.

I don't understand the purpose of the changes in oolite-cloaking-device-mission.js either. Why put everything in a closure function? Why pull out the definition of the callback function used in the populator? What is the meaning of creating a variable pointing to the function whose scope contains it (var 'that'), and what is "var setPopulator = (that.setPopulator = that.setPopulator || system.setPopulator);" doing?

At line 1190 of oolite-populator.js (your version), the property is missing an 's' at the end:

if (random() < repop_freq_In.thargoidStrikes)

This was very recently fixed in the core.

cag
Dangerous
Dangerous
Posts: 98
Joined: Fri Mar 17, 2017 1:49 am

Re: Strict mode warnings

Post by cag »

Milo wrote:
Thu Jul 09, 2020 1:16 am
why move "use strict"; inside the scope of the this.activated function
To satisfy JSLint, which kept complaing 'Use the function form of "use strict". For scripts with more than one function, this would become tedious, so we wrap it all in a function.
Svengali wrote:
Fri Jul 21, 2017 8:37 am
You don't need to do it for every function. Just wrap your code in a function. Please also note that the jshint and the global settings will vary from script to script - sometimes you don't need them at all.

Code: Select all

/* jshint <your options> */
/* global <your global vars> */
(function(){
"use strict";
<your code>

}).call(this);
I never did find out if it matters much to Oolite's JS interpreter. It does no harm and I don't have to sift through all of those errors to find the ones that matter.
Milo wrote:
Thu Jul 09, 2020 1:16 am
why name the function with 'this.activated = function activated()' instead of just 'this.activated = function()'
I got real annoyed of messages saying '<anonymous function>'. This way, I get the name I've given to the function . The real benefit comes when you're profiling: it gives line numbers but with a named function, it's much easier to interpret. The name between 'function' and '()' is just that, a name - can be anything you want; it has no other effect than to make messages less annoying. :)

In this particulr case, it probably doesn't matter. I just got in the habit of naming all my functions.
Milo wrote:
Thu Jul 09, 2020 1:16 am
I don't understand the purpose of the changes in oolite-cloaking-device-mission.js either. Why put everything in a closure function?
Cleaner code, in that "use strict" appears only once. I've seen some that wrap the entire script, including this.name on the 1st line. We're not a very consistent bunch.
Milo wrote:
Thu Jul 09, 2020 1:16 am
Why pull out the definition of the callback function used in the populator? What is the meaning of creating a variable pointing to the function whose scope contains it (var 'that'), and what is "var setPopulator = (that.setPopulator = that.setPopulator || system.setPopulator);" doing?
I'll answer these in reverse order. The third was all Day's fault
viewtopic.php?p=257444#p257444
We're creating a property for the function called 'setPopulator'. The first time the script is invoked, it fetches a reference to the 'system.setPopulator' function and saves it in this new property. All subsequent calls do not have to fetch it, as it's stored locally. Why you ask? Speed. Maps (aka dictionaries) are much faster than arrays to search but they still take time. worldScripts is the worst as it's HUGE. When profiling timers & frame callbacks, these property 'gets' can account for much of the execution time. And it's a waste, as the reference is static - the function is always at that same reference.

The name of the var 'that' is nothing special, just thematically similar to this. Having named the function 'systemWillPopulate', I could have just used that instead. The reason we use a second variable is so we can cut and paste these reference caches from function to function. All we have to change is the value assigned to 'that'.

As for pulling out the callback function, besides being good practice and easier to read, it was a side effect of the next line

Code: Select all

var ents = (that.ents = that.ents || {});
where 'ents' is another function property containing an object which we reuse. In the original, every call to that function would create a new object which then gets discarded. Another pet peeve of mine is those little freezes that seem to happen at the worst possible moment. That's JS performing garbage collection. JS is much easier to code in than, say, C, where you're responsible for memory management. The price we pay is garbage collection. This is performed whenever you launch or exit witch space, when you'll never notice it. But with more oxp's loaded, some will be generating garbage, leading to these mini-strokes oolite has.

All of this probably has little if any effect here, as this function is rarely called. The main purpose of doing this consistently across all the scripts was to have a ready example for new oxp authors to reference.


FYI, I just installed the latest nightly and there were a couple tweaks in oolite-populator.js, so you'll have to download that zip file again, same line.

https://www.dropbox.com/s/cudwuq8d56ep ... .zip?dl=0

=====================================================================
At line 1190 of oolite-populator.js (your version), the property is missing an 's' at the end:

if (random() < repop_freq_In.thargoidStrikes)

This was very recently fixed in the core.
That was fast. Here I am about to post this reply to tell you the same thing. That was one of two changes to that script.
"Better to be thought a fool, boy, than to open your trap and remove all doubt." - Grandma [over time, just "Shut your trap... fool"]
"The only stupid questions are the ones you fail to ask." - Dad
How do I...? Nevermind.

User avatar
Day
---- E L I T E ----
---- E L I T E ----
Posts: 528
Joined: Tue Mar 03, 2015 11:35 am
Location: Paris

Re: Strict mode warnings

Post by Day »

:mrgreen:

User avatar
Milo
Deadly
Deadly
Posts: 236
Joined: Mon Sep 17, 2018 5:01 pm

Re: Strict mode warnings

Post by Milo »

cag, with your scripts, I still get this error:

Code: Select all

Exception: TypeError: leader.group is null
    Active script: Oolite Police AI 1.89
    oolite-priorityai.js, line 1189:
    			gs = leader.group.ships;

cag
Dangerous
Dangerous
Posts: 98
Joined: Fri Mar 17, 2017 1:49 am

Re: Strict mode warnings

Post by cag »

Thanks for the tip. New version uploaded, same link

https://www.dropbox.com/s/cudwuq8d56ep ... .zip?dl=0
"Better to be thought a fool, boy, than to open your trap and remove all doubt." - Grandma [over time, just "Shut your trap... fool"]
"The only stupid questions are the ones you fail to ask." - Dad
How do I...? Nevermind.

User avatar
Milo
Deadly
Deadly
Posts: 236
Joined: Mon Sep 17, 2018 5:01 pm

Re: Strict mode warnings

Post by Milo »

Another one:

Code: Select all

Exception: TypeError: this.ship.group is null
    Active script: Oolite Escort AI 1.89
    oolite-priorityai.js, line 6100:
    	if (ally == this.ship.group.leader)

cag
Dangerous
Dangerous
Posts: 98
Joined: Fri Mar 17, 2017 1:49 am

Re: Strict mode warnings

Post by cag »

You're starting to freak me out, dude. While that line 6100 was from the core distribution, the same error is in mine too. That's been there for YEARS! :shock: How are you finding these?

New version uploaded, same link

https://www.dropbox.com/s/cudwuq8d56ep ... .zip?dl=0
"Better to be thought a fool, boy, than to open your trap and remove all doubt." - Grandma [over time, just "Shut your trap... fool"]
"The only stupid questions are the ones you fail to ask." - Dad
How do I...? Nevermind.

User avatar
Milo
Deadly
Deadly
Posts: 236
Joined: Mon Sep 17, 2018 5:01 pm

Re: Strict mode warnings

Post by Milo »

I'm cursed, and/or I have a lot of OXPs that populate my systems heavily and expose edge cases, and/or I'm running with the debug console and pedantic mode, so I see all the errors, and/or I'm cursed.

Post Reply