Page 1 of 1

equipmentStatus

Posted: Sun Jan 23, 2011 3:06 am
by Screet
Hi,

looks like I haven't read the wiki well enough, didn't see it due to my own installation and finally only became aware of it by a bug report.

The wiki says:

Code: Select all

function equipmentStatus(equipmentType : equipmentInfoExpression) : String

Tests whether the specified type of equipment is installed, and whether it is functioning. Returns one of the following strings: "EQUIPMENT_OK", "EQUIPMENT_DAMAGED", "EQUIPMENT_UNAVAILABLE". (Prior to Oolite 1.74, this method was only available for the player’s ship.)

Note: by design, this method will throw an exception if called with an equipment type that does not exist. To test whether an equipment type exists, use EquipmentInfo.infoForKey(), which will return null for undefined equipment. 
Yes, it's possible to work around that. However, I don't understand why this is this way - can the request be answered faster if it doesn't have to report "EQUIPMENT_UNAVAILABLE" when the piece of EQ being tested is not around?

The reason is that the oxp has to check for installed&working EQ from other -optional- oxps. So I would have to code it in a slower way: first check wether that oxp is installed and then check wether it is working. It appears better to me would the code return EQUIPMENT_UNAVAILABLE if that piece of EQ is not available (because the oxp is not installed).

Just curious, no real problem. But it would save the time to double-check...

Screet

Re: equipmentStatus

Posted: Sun Jan 23, 2011 11:32 am
by JensAyton
As I see it, the most likely use case for equipmentStatus() is to work with equipment defined by the same OXP, and the most likely reason to hit the exception is a misspelled equipment identifier. Because it’s easy to be blind to your own typos, allowing this leads to a frustrating situation where apparently-correct code just doesn’t work. On the other hand, when you hit the exception the way you are it’s obvious you need to look for another way to handle your situation.

My general feeling as a programmer is that relaxed interfaces are nice when you’re using them right, but they aren’t worth the pain when you use them wrong and they don’t tell you why.

On the other hand, it is a bit odd that the other query method, canAwardEquipment(), simply returns false for unknown types but equipmentStatus() throws an exception. Returning “EQUIPMENT_UNKNOWN” instead might be a reasonable compromise.

Re: equipmentStatus

Posted: Mon Jan 24, 2011 11:30 am
by Screet
Ahruman wrote:
On the other hand, it is a bit odd that the other query method, canAwardEquipment(), simply returns false for unknown types but equipmentStatus() throws an exception. Returning “EQUIPMENT_UNKNOWN” instead might be a reasonable compromise.
Yes, that is what did hit me: I've been using the outdated interface before which also didn't throw an exception. For the script, I've worked around this by initially checking for the existence of the other oxp and storing that info in a variable so that I can later check wether the oxp is installed and only then check wether the equipment is working.

However, were this to be updated at some point, it might be a good idea to do this: add a log variable that tells wether to log a warning for unknown equipment or not and always return unknown for the calls, instead of throwing an exception.

That way, an oxp developer could easily use the log variable to see wether he made a typo (or is testing without the other oxp) and it would work in any such cases, but players won't have to worry about info about oxp's that are not installed.

Of course, that's only "nice to have" and adds improved consistency, helping both type of people: players and oxp authors.

Screet

Re: equipmentStatus

Posted: Tue Jan 25, 2011 7:33 pm
by JensAyton
I’m not a fan of the optional warning thing, but I’ve changed equipmentStatus() to return "EQUIPMENT_UNKNOWN" when the argument is a string that’s not a known equipment identifier (it will still throw an exception if you pass a number or something dumb like that). Using a distinct value rather than "EQUIPMENT_UNAVAILABLE" should make it easy enough to track down bad identifiers.