shipSpawned and main stations (rev 4496)

[Commander McLane]

While testing NPC-shields.oxp I noticed that the shipSpawned world script handler doesn't fire for main stations. Other stations yes, but not main stations. Feature or bug?

Small test case. Put this in a world script:

Code: Select all

this.shipSpawned = function(ship)
{
    ship.script.wasSpawned = true;
}

Then target any ship and type

Code: Select all

PS.target.script.wasSpawned

in the console. It will return true for all entities, except main stations.

[Eric Walch]

While testing NPC-shields.oxp I noticed that the shipSpawned world script handler doesn't fire for main stations. Other stations yes, but not main stations. Feature or bug?

I don't know why the main station is treated different. I guess its a bug. For the shipSpawned to trigger, a ship must have status: STATUS_IN_FLIGHT, STATUS_LAUNCHING or STATUS_BEING_SCOOPED). For some reason the main station is the only one with status: STATUS_ACTIVE.

In the code is STATUS_ACTIVE only used for subentities like turrets and planets. The main station is the exception. The code for setting the main station status reads:

Code: Select all

[a_station setStatus:STATUS_ACTIVE];	// For backward compatibility. Might not be needed.

But I don't know what backward compatibility is mend. I guess IN_FLIGHT would be okay as I am not aware of problems with other stations.

[JensAyton]

Code: Select all

[a_station setStatus:STATUS_ACTIVE];	// For backward compatibility. Might not be needed.

But I don't know what backward compatibility is mend. I guess IN_FLIGHT would be okay as I am not aware of problems with other stations.

The comment came from r2905 (kaks) during refactoring. (Thanks, svn blame!) Before that, all stations/carriers had STATUS_ACTIVE. This dates back to r170 (winston), with the comment “merge from OS X r1030”. That source tree no longer exists as far as I know. So basically, no-one actively involved now knows, and I doubt Giles remembers.

It’s quite likely there’s no good reason, but at this point I’d rather not change the scan class before MNSR. Better to change the shipSpawned test to include STATUS_ACTIVE && !isSubEntity.

[Kaks]

Ah yes!

When I refactored that code it did strike me as odd that the code gave all stations STATUS_ACTIVE. I suppose that the original rationale for ACTIVE as opposed to IN_FLIGHT was that stations were not supposed to move about - kind of a moot point though, since stationary splinters would have STATUS_IN_FLIGHT, without having any means of propelling themselves.

My first draft of that refactoring had all stations, including the main one, being set as STATUS_IN_FLIGHT.

However, I couldn't rule out the possibility that one 1.65 era OXP would be checking for a station using STATUS_ACTIVE, so I 'provisionally' kept the old status for the one station that was guaranteed to always be in a solar system.


I actually meant to ask wiser people about it, but it -err- kind of slipped my mind...



Edit: looked back through SVN and remembered a lot more details! Hey, it all happened in December 2009!

At one point I did set all stations & carriers to STATUS_ACTIVE just to produce the same result as before refactoring, but it did interfere with some spawn scripts attached to carriers, and IIRC it was Eric who saw that problem - it was there before the refactoring, but testing if I'd broken anything brought to light a bug no-one had noticed...

Anyways, as I said on r2905's comment:

- Apart from the main stations, which are still spawned with STATUS_ACTIVE, all other stations are now spawned with STATUS_IN_FLIGHT. Should fix some breakages.

Ah, the magic of SVN!

[JensAyton]

I note that the comment where shipSpawned() is called says: “we only want shipSpawned to be triggered if this is a ship (including carriers and cargo pods), not for a stationary err... station!” The motivation for this isn’t obvious, but I believe this historically applied to launch_actions.

I almost checked in a fix that enables shipSpawned() for main stations, but doesn’t invoke launch_actions for any station. The problem with this is that it will disable launch_actions for carriers. Is there a reasonable way to distinguish between “carriers” and “stations” from a script?

[TGHC]

Is there a reasonable way to distinguish between “carriers” and “stations” from a script?

<puts on techy hat>
They're spelt differently.
<looks round in panic for an escape route>

[Eric Walch]

Is there a reasonable way to distinguish between “carriers” and “stations” from a script?

The only way I found in the past was looking at maxSpeed. It is not fool-proof, but when zero, it must be a station. When not zero, it still could be a station, although very unlikely. A station with a non-zero maxSpeed has the problem that when a ship bumps into it, it could start to float in space. So, probably any station with maxSpeed > 0 is intended to be a carrier.
Thrust is no option as even stations need a thrust value to keep them on the spot in case of collisions.

[JensAyton]

Of course, what I should really do is work out whether 1.65 actually made that distinction correctly and, if so, how. *sigh*

[Thargoid]

Isn't there an isCarrier key, or am I thinking of shipdata.plist there?

Personally I'd go with Eric's maxSpeed thought, or perhaps if it's visible to script then additionally check the rotation speed of the entity (as Stations can rotate but carriers cannot). But again that's not foolproof as not all stations do rotate.

[JensAyton]

isCarrier/is_carrier in shipdata.plist is just a way of flagging something as a station/carrier, not distinguishing between them. There isn’t really a formal distinction.

As far as I can tell, only the main station has STATUS_ACTIVE in 1.65; other stations start in STATUS_IN_FLIGHT and hence have launch_actions. I’ve checked in a fix where shipSpawned() fires for the main station, but launch_actions are suppressed.

[Zireael]

As far as I can tell, only the main station has STATUS_ACTIVE in 1.65; other stations start in STATUS_IN_FLIGHT and hence have launch_actions. I’ve checked in a fix where shipSpawned() fires for the main station, but launch_actions are suppressed.

I don't know a thing about Oolite coding, but why not make a fix in the next nightly that sets it to STATUS_IN_FLIGHT for main station? Let's make it consistent...

[JensAyton]

I don't know a thing about Oolite coding, but why not make a fix in the next nightly that sets it to STATUS_IN_FLIGHT for main station? Let's make it consistent...

Because it’s unclear what the effects of that would be, and we’re not experimenting at the moment.