Can't unmark systems from ship scripts [solved]

[Eric Walch]

Lovecats unmarks two at once a few times. Though it does that from a ship script so would need adjusting anyway.

On the other hand, Feudal States already expects it to be working in the way that setInstructions does, with the option to take an array to set more than one system at once. It works, too, because of how type conversion works in the JS libraries.

The parameter is only a convenience, of course, so if the code is to be rewritten anyway:

Code: Select all

// in world script
this._unmarkSystem = function(id) {
  mission.unmarkSystem(id);
}
// in ship script
this.shipDied = function() {
  worldScripts.foo._unmarkSystem(system.ID);
}

I'd rather not change the meaning of markSystem to break any more code than the unavoidable ship script case. mission.markSystemAs(id,script) is a possibility.

I was already sceptical when you proposed it as I never could make this type of stuff working. By calling the worldScript from within the ship script, Oolite still sees the ship script as the active script. Today I finally had the chance to test it with a nighty (r5144). I rewrote Lovecats, with above code, but without success. The systems stay marked. I expect a lot of oxps will not be able to unmark systems in the future.

In the past I had similar problems and the only solution I could come up with was starting a timer with zero seconds delay in the worldscript. When that timer fires, it does it from the worldscript and the active script is than indeed the intended one. I think that procedure will also be needed in future to unmark the destinations from ship scripts.

[Wildeblood]

Why is there a need for this wrapper function in the world script?

Code: Select all

// in world script
this._unmarkSystem = function(id) {
  mission.unmarkSystem(id);
}
// in ship script
this.shipDied = function() {
  worldScripts.foo._unmarkSystem(system.ID);
}

You should be able to just write this in the ship script:-

Code: Select all

// in ship script
this.shipDied = function() {
  worldScripts["foo"].mission.unmarkSystem(system.ID);
}

[Eric Walch]

I now tried it with timers and even than it does not work. I have made
Shipscript ship

Code: Select all

this.shipLaunchedEscapePod = function ()
{
    worldScripts["wedding"].$markSystem(83, false);
    worldScripts["wedding"].$markSystem(114, false);
}

Shipscript pod

Code: Select all

this.shipWasScooped = function (scooper)
{
    
    worldScripts["wedding"].$markSystem(16, true);
}

Worldscript

Code: Select all

this.startUp = function() {
    mission.markSystem(83);
    mission.markSystem(114);
};

this.$markSystem = function (systemID, mark) {
    if (mark) {
        worldScripts.wedding["lovecatsTimer"+systemID] = new Timer(worldScripts.wedding, function() {mission.markSystem(systemID);}, 0.1);
        log(this.name, "Marked system: "+systemID);
    } else {
        worldScripts.wedding["lovecatsTimer"+systemID] = new Timer(worldScripts.wedding, function() {mission.unmarkSystem(systemID);}, 0.1);
        log(this.name, "Unmarked system: "+systemID);
    }
};

resulting latest log

Code: Select all

[wedding]: Unmarked system: 83
[wedding]: Unmarked system: 114
[wedding]: Marked system: 16

I also removed all 'this' by an explicit worldscript in the timer.

All without success. The worldscript does mark the systems on startup. But the shipscript fails to unmark them. Than the pod-script is able to mark a new destination.

And from the save file I made afterwards:

Code: Select all

	<key>mission_destinations</key>
	<dict>
		<key>lovecat_escapepod</key>
		<array>
			<integer>16</integer>
		</array>
		<key>wedding</key>
		<array>
			<integer>83</integer>
			<integer>114</integer>
		</array>
	</dict>

Clearly the destinations are still linked to the ship scripts.

[cim]

I was already sceptical when you proposed it as I never could make this type of stuff working. By calling the worldScript from within the ship script, Oolite still sees the ship script as the active script.

Okay, that needs some more work, then. I'll have a look at it this weekend if I don't get time before.

How about this as an option, which lets us have a bit more flexibility:
(un)markSystem currently takes a list of parameters, which are currently all ints, but could either be ints or objects. An int is then equivalent to the object {system: int}. Then we could add other parameters to the objects too.

Code: Select all

mission.markSystem({
    system: 37,  // required
    name: "my_script",  // optional, defaults to this.name of the active script
    marker_style: "MARKER_DIAMOND", // optional, defaults to "MARKER_X"
    marker_color: "greenColor", // optional, defaults to redColor
    marker_scale: 1.25, // optional, defaults to 1.0, range 0.0..2.0
});

I think it'll be useful to be able to differentiate markers if more than one thing is marking a system. Giving cargo contracts and passenger contracts a different marker to the default might also be useful to have.

[Eric Walch]

How about this as an option, which lets us have a bit more flexibility:
(un)markSystem currently takes a list of parameters, which are currently all ints, but could either be ints or objects.

Using objects is certainly be a great idea for a change. Using objects won't be backward compatible but the benefit is worth it and it will be easy expandable in the future if there is a need.

[cim]

How about this as an option, which lets us have a bit more flexibility:
(un)markSystem currently takes a list of parameters, which are currently all ints, but could either be ints or objects.

Using objects is certainly be a great idea for a change. Using objects won't be backward compatible but the benefit is worth it and it will be easy expandable in the future if there is a need.

I'll have to try it to see, but I'm fairly sure it's possible to detect if a JS parameter is an object or an int, and act accordingly. JSVAL_IS_INT looks promising, anyway.

[JensAyton]

I'll have to try it to see, but I'm fairly sure it's possible to detect if a JS parameter is an object or an int, and act accordingly. JSVAL_IS_INT looks promising, anyway.

Sort of. In some circumstances, numbers can be autoboxed as Number objects. Also, note that there is no such thing as an “int” in JavaScript; the INT and DOUBLE internal types are different representations of a single “primitive number” nominal type.

A rough outline of correct handling would be:

Code: Select all

int32_t numberValue;
if (JSVAL_IS_OBJECT(value) && JS_GetClass(context, JSVAL_TO_OBJECT(value)) != [[OOJavaScriptEngine sharedEngine] numberClass])
{
    // Handle object form here
}
else if (JS_ValueToInt32(context, value, &numberValue))
{
    // Handle number case here
}
else
{
    OOJSReportBadArguments(...);
    return NO;
}

It’s technically possible but highly unlikely that someone’s calling markSystem() with a custom object which has a toValue() method that returns a number. This case will currently work since JS_ValueToInt32() will do the necessary coersion. This degree of sophisticated ugliness is pretty rare in OXPs, though. :-)

Also, please use camelCase for the object keys.

[cim]

Code: Select all

name: "my_script",  // optional, defaults to this.name of the active script

It occurs to me now that if I make the default for this be the legacy script name, then the entire thing should be fully backward-compatible with 1.76 code, which is probably a better approach.

Ahruman: thanks - will do something like that, then. If I do the JS_ValueToInt32 branch first, that should catch the case where someone is passing a strange object that ends up as an int in, shouldn't it (like an object-boxed string from missionVariables?), leaving only the case where someone has an object which mysteriously already has the right properties but has a toValue that just returns the ID...

[JensAyton]

If I do the JS_ValueToInt32 branch first, that should catch the case where someone is passing a strange object that ends up as an int in, shouldn't it (like an object-boxed string from missionVariables?)

It seems like that will work, yes. (Note that mission variables that are number literals are read as unboxed numbers, though.)

[Eric Walch]

How about this as an option, which lets us have a bit more flexibility:
(un)markSystem currently takes a list of parameters, which are currently all ints, but could either be ints or objects. An int is then equivalent to the object {system: int}. Then we could add other parameters to the objects too.

Code: Select all

mission.markSystem({
    system: 37,  // required
    name: "my_script",  // optional, defaults to this.name of the active script
    markerShape: "MARKER_DIAMOND", // optional, defaults to "MARKER_X"
    markerColor: "greenColor", // optional, defaults to redColor
    markerScale: 1.25, // optional, defaults to 1.0, range 0.5..2.0
});

I just played a bit with it today (32°C outside and the computer room is the only one with airco). The new marking works great.

I rewrote parts of UPS_Courier to use the long_range_chart as mission background, to replace my complex way of doing something similar. UPS mission destinations have now their own shape and colour.

I was just wondering what to do with missions accepted in the old version and finished with the new version. They will stay on until the destination are auto-cleared on jumping to a new galaxy. I was looking for ways to recognise this situation. One way would be if the unmark option would return false if there was nothing to unmark.

I think you planned that we use mission.markedSystems. It is just that I can't get it working. In the wiki you write that it is an array of destination-objects, but when I look in the code, I see that it is an dictionary with destination-objects. However, I see no way to access the keys. I thought it should be: mission.markedSystems["systemID-my_oxp"] (Same key as the save file) But no luck

[cim]

I think you planned that we use mission.markedSystems. It is just that I can't get it working. In the wiki you write that it is an array of destination-objects, but when I look in the code, I see that it is an dictionary with destination-objects.

Internal storage is dictionary of dictionaries, but it should be translating to an array of dictionaries for JS access, though.

Here's what I get with the debug console

Code: Select all

> mission.markSystem(32,64,128)
> mission.markSystem({system: 16, name: "test", markerShape: "MARKER_PLUS"})
> mission.markedSystems()
[{
    markerScale: 1,
    system: 128,
    name: "__oolite_legacy_destinations",
    markerShape: "MARKER_X",
    markerColor: "redColor"
}, {
    markerScale: 1,
    system: 32,
    name: "__oolite_legacy_destinations",
    markerShape: "MARKER_X",
    markerColor: "redColor"
}, {
    markerScale: 1,
    system: 16,
    name: "test",
    markerShape: "MARKER_PLUS",
    markerColor: "redColor"
}, {
    markerScale: 1,
    system: 64,
    name: "__oolite_legacy_destinations",
    markerShape: "MARKER_X",
    markerColor: "redColor"
}]
> mission.markedSystems()[1]
{
    markerScale: 1,
    system: 32,
    name: "__oolite_legacy_destinations",
    markerShape: "MARKER_X",
    markerColor: "redColor"
}

[Eric Walch]

I suddenly see what I did wrong. I was accessing it as property. I now see that it is a method. Thanks.

Edit: Okay, I now got it working in UPS. On first chance UPS now replaces all legacy markers of active missions with a new one. (Now I have to decide for a final shape and colour)

[JensAyton]

:oops: I suddenly see what I did wrong. I was accessing it as property. I now see that it is a method.

This does seem rather inconsistent.

[cim]

It does. Since I can't remember why I did it that way, changed to a property in r5217 before anyone other than Eric starts using it...

[Cody]

Since I can't remember why I did it that way

Oh, that's something I find happening to me far too often lately... heh.