Page 11 of 16

Posted: Wed Oct 07, 2009 4:04 pm
by CheeseRedux
After a bit of trial and error, the problem seems to be solved.

Having line 568 as
hasShipyard = ("score_number greaterthan 1") ;
causes crash.

Trying
hasShipyard = 1 ;
makes the entire station disappear from the system!

Same thing with
hasShipyard = 2 ;
which I tried since it says greater than 1 :)

EDIT
This is the last lines of the log when the station was missing:
[plist.homebrew.parseError]: ***** Property list parser error: expected root element tokenization to be NSArray, but got (nil).
[script.addShips.failed]: ***** SCRIPT ERROR: in <anonymous actions>, addShipsAtPrecisely: could not add 1 ships with role 'repaired-buoy-station'
/EDIT

However, both
hasShipyard = 0 ;
and
hasShipyard = yes ;
removes the CTD.

I have no clue as to what else might be going on here.

I'll complete the bugreport and append the fix as well.

Posted: Wed Oct 07, 2009 4:30 pm
by Eric Walch
CheeseRedux wrote:
However, both
hasShipyard = 0 ;
and
hasShipyard = yes ;
removes the CTD.

I have no clue as to what else might be going on here.
yes is the correct value for open step, but for most keys you can also use 1. That hasShipyard = 1 ; had probably an typo somewhere n the code as the error and not adding means it could not read the plist at all.

Anyway, I just updated the oxp to version 1.02.5.

(I deliberately waited with the 1.02.4 release to see if there were problems with it under 1.73. But Murphy says: No matter how long you wait, bugs only emerge shortly after release)

Posted: Wed Oct 07, 2009 4:49 pm
by CheeseRedux
Downloaded and tested both as only OXP and alongside my regular batch of stuff.
Seems to work perfectly.
(It's a good thing you oriented that docking port facing the station. All this high speed docking with injectors burning to the last minute would be tricky if I had to maneuver as well - especially those times with hostiles on my tail.)

Oh, one small thing:
The fix I applied manually fixed the crash, but gave no shipyard - your new download has ships for sale. Goody, goody!

Posted: Wed Oct 07, 2009 6:08 pm
by Svengali
If I get this right the following is treated as fuzzy boolean. So it can happen that 1 is only a probability and can result in 0.

Code: Select all

hasShipyard = 1;
I'm often using fuzzy boolean values to spread the possibilities a little bit even if they are at the highest possible valid value.

Oh and spaces between value and semicolon are maybe another point to be verified.
CheeseRedux wrote:

Code: Select all

hasShipyard = 0 ;
For v1.74 we'll maybe step to has_shipyard instead.

Posted: Thu Oct 08, 2009 1:08 am
by zevans
CheeseRedux wrote:
this also means that I consistently try to dock with anything (new) I can dock with,
I know people with dogs like that... :lol:

Posted: Thu Oct 08, 2009 7:51 am
by Kaks
Thanks, another_commander posted a fix in trunk to stop this crash for future versions of Oolite. Keep up the good work finding these bugs! :)

Posted: Thu Oct 08, 2009 8:03 am
by another_commander
Just as an additional note, it appears that the syntax for the conditions in cases like this has been changed in 1.73.4. Doing a

Code: Select all

hasShipyard = ("score_number greaterthan 1");
will not work anymore. What does work is this:

Code: Select all

hasShipyard = ((1, "score_number greaterthan 1", score_number, 3, ((0, 1))));
It is quite a bit more complex than before, so I will just post here the explanation and comments given in the code as to what the above means.
The source code comments wrote:
Test a script condition sanitized by OOLegacyScriptWhitelist.

A sanitized condition is an array of the form:
(opType, rawString, selector, comparisonType, operandArray).

opType and comparisonType are NSNumbers containing OOOperationType and
OOComparisonType enumerators, respectively.

rawString is the original textual representation of the condition for
display purposes.

selector is a string, either a method selector or a mission/local
variable name.

operandArray is an array of operands. Each operand is itself an array
of two items: a boolean indicating whether it's a method selector
(true) or a string (false), and a string.

The special opType OP_FALSE doesn't require any other elements in the
array. All other valid opTypes require the array to have five elements.

For performance reasons, this method assumes the script condition will
have been generated by OOSanitizeLegacyScriptConditions() and doesn't
perform extensive validity checks.
The enumeration for opType is:
OP_STRING, // 0
OP_NUMBER, // 1
OP_BOOL, // 2
OP_MISSION_VAR, //3
OP_LOCAL_VAR, // 4
OP_FALSE, // 5

The enumeration for comparisonType is:
COMPARISON_EQUAL, // 0
COMPARISON_NOTEQUAL, // 1
COMPARISON_LESSTHAN, // 2
COMPARISON_GREATERTHAN, // 3
COMPARISON_ONEOF, // 4
COMPARISON_UNDEFINED // 5

Posted: Thu Oct 08, 2009 8:32 am
by another_commander
After posting my previous message, I took a moment and thought "Wait a minute, it cant' t REALLY be this way, that's just too complicated". So I had a quick look at an idea that came up and it seems that all the above is a description of the internals of the processing done on coditional arrays, that should be already taken care of with the legacy script whitelisting. In reality, this should work just fine:

Code: Select all

hasShipyard = ("score_number greaterthan 1");
I will add the complete fix to trunk in a while and leave the earlier message in for anyone who wants to have some more in-depth info, but bottomline is that the conditions will not have to be changed.

Posted: Thu Oct 08, 2009 8:44 am
by pmw57
I would like to take this opportunity to put my hand forward to perform a scripted conversion of this code from openstep to javascript.

This should also help to make clearer any other issues that may exist in the legacy code.

Posted: Thu Oct 08, 2009 9:13 pm
by JensAyton
Just to be clear, there is no context in which specifying the sanitized format in a plist should work, let alone be the recommended approach. That’s sort of the point.

Posted: Thu Oct 08, 2009 9:15 pm
by JensAyton
pmw57 wrote:
I would like to take this opportunity to put my hand forward to perform a scripted conversion of this code from openstep to javascript.
There’s a partial implementation of a legacy-to-JavaScript converter, but I’ve decided against transparent conversion. Further details on the reasoning behind that depend on stuff that hasn’t been announced yet, and won’t be for a while.

At one point, an advantage of that would also have been to find errors in legacy code by parsing it up front, but much of that benefit is provided by the sanitising mechanism as of 1.73 (except in these cases which I’d missed).

Posted: Thu Oct 08, 2009 9:29 pm
by pmw57
Ahruman wrote:
pmw57 wrote:
I would like to take this opportunity to put my hand forward to perform a scripted conversion of this code from openstep to javascript.
There’s a partial implementation of a legacy-to-JavaScript converter, but I’ve decided against transparent conversion. Further details on the reasoning behind that depend on stuff that hasn’t been announced yet, and won’t be for a while.
I agree, automatic conversion can be troublesome.

How do you feel about back-breaking labour-intensive hand-coded conversions instead?

Posted: Thu Oct 08, 2009 9:45 pm
by JensAyton
pmw57 wrote:
How do you feel about back-breaking labour-intensive hand-coded conversions instead?
I’m in favour of someone else doing them. ;-)

I may finish the converter, but for use as an offline tool. In most cases, though, the resulting code would still require a complete rewrite from the “tickle”-based approach to an event-driven one. Most types of events can’t be reliably extracted automatically. (The exceptions would be launch and entered-system checks, since these are guaranteed to only be evaluated once per relevant event.)

Posted: Fri Oct 09, 2009 9:02 am
by Screet
How about to extend the GRS service?

I've just flown towards "S" and then it vanished. Some stupid navy pilot obviously destroyed that buoy!

Thus, why not also replace navy "S" buoys if GRS and gal navy oxp are active? ;)

Anyway...those navy and police pilots really are stupid. I just launched and the leviathan platform went hostile. Some super-tiger police pilot also was attacking it. After I shot it down, he fired his injectors and did fly into the planet. That stupid guy even complained about his expensive ship being destroyed...they really should send them to basic flight training before sending them out there! ;)

Screet

Posted: Fri Oct 09, 2009 9:57 am
by Commander McLane
That would depend entirely what the "S" beacon code stands for. Acording to the Wiki two known uses of that letter are the Navy SecCom Station (Galactic Navy OXP) and the Special Branch Orbital Headquarters (Assassins OXP). None of these is a buoy, but both are stations, as far as I know.

So why should the Buoy Repair Services replace a blown-up station with a buoy?