shipSpawned and main stations (rev 4496)

For test results, bug reports, announcements of new builds etc.

Moderators: winston, another_commander, Getafix

Post Reply
User avatar
Commander McLane
---- E L I T E ----
---- E L I T E ----
Posts: 9520
Joined: Thu Dec 14, 2006 9:08 am
Location: a Hacker Outpost in a moderately remote area
Contact:

shipSpawned and main stations (rev 4496)

Post by 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.
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Re: shipSpawned and main stations (rev 4496)

Post by Eric Walch »

Commander McLane wrote:
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.
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Re: shipSpawned and main stations (rev 4496)

Post by JensAyton »

Eric Walch wrote:

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.
User avatar
Kaks
Quite Grand Sub-Admiral
Quite Grand Sub-Admiral
Posts: 3009
Joined: Mon Jan 21, 2008 11:41 pm
Location: The Big Smoke

Re: shipSpawned and main stations (rev 4496)

Post by 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... :oops:



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! :)
Hey, free OXPs: farsun v1.05 & tty v0.5! :0)
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Re: shipSpawned and main stations (rev 4496)

Post by 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?
User avatar
TGHC
---- E L I T E ----
---- E L I T E ----
Posts: 2157
Joined: Mon Jan 31, 2005 4:16 pm
Location: Berkshire, UK

Re: shipSpawned and main stations (rev 4496)

Post by TGHC »

Ahruman wrote:
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>
The Grey Haired Commander has spoken!
OK so I'm a PC user - "you know whats scary? Out of billions of sperm I was the fastest"
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Re: shipSpawned and main stations (rev 4496)

Post by Eric Walch »

Ahruman wrote:
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.
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Re: shipSpawned and main stations (rev 4496)

Post by JensAyton »

Of course, what I should really do is work out whether 1.65 actually made that distinction correctly and, if so, how. *sigh*
User avatar
Thargoid
Thargoid
Thargoid
Posts: 5528
Joined: Thu Jun 12, 2008 6:55 pm

Re: shipSpawned and main stations (rev 4496)

Post by 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.
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Re: shipSpawned and main stations (rev 4496)

Post by 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
---- E L I T E ----
---- E L I T E ----
Posts: 1396
Joined: Tue Nov 09, 2010 1:44 pm

Re: shipSpawned and main stations (rev 4496)

Post by 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...
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Re: shipSpawned and main stations (rev 4496)

Post by JensAyton »

Zireael wrote:
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.
Post Reply