*Maybe* bug in priority AI homeStation function

[phkb]

I've been playing around with AI's and came across this oddity. The "homeStation" function in priority AI will return (and potentially set) a ship's home station. Is starts off like this:

Code: Select all

PriorityAIController.prototype.homeStation = function() 
{
	if (!this.__ltcache.oolite_homeStation || this.__ltcache.oolite_homeStation === null)
	{
		return null;
	}

Now, I've checked, and I can find nowhere else in the code where this.__ltcache.oolite_homeStation is set. It happens only in this function, at points after this opening code snippet. That's important because of the logic in play here. And that's where I think there might be a logical fault.

So, my reading of this code is:
If the __ltcache.oolite_homeStation property does not exist (or potentially, is equal to false or zero) OR the __ltcache.oolite_homeStation property is exactly equal to null, then return a value of null.

Which seems fairly straightforward. But, if the only place oolite_homeStation gets set is in code that comes after this bit, then oolite_homeStation will never get set. Which means any other code that relies on having a value in oolite_homeStation will not work as planned.

Is my reading of this correct? Is my logic sound?

I think the first clause of that if statement is unnecessary. But I wanted to get some other opinions before I check in a code change.

[Alnivel]

Your commit 2f9088b added !this.__ltcache.oolite_homeStation bit, but before this the function worked somewhat like this:

if oolite_homeStation is null return null;
if oolite_homeStation is already set and the value is still valid return the cached value;
otherwise (if oolite_homeStation is not set yet or has become invalid) look for and cache the new home station (or null if there is none);


I've only now noticed that you want to remove only the first clause, not the if branch entirely, sorry, so probably my reasoning above was unnecessary, but whatever.
Your reading seems correct to me and I think too that the first clause should be removed.

[phkb]

Your commit 2f9088b added !this.__ltcache.oolite_homeStation bit

Oh, don't tell me...

Oh, biscuits. I added that code...



Excuse me while I go somewhere and shoot myself in the foot a few more times...

[Cody]

<sniggers>

[phkb]

Yep, move along people, nothing to see here.