Just a comment on a line of code

[Massively Locked]

The line I wrote is:

if (player.ship.dockedStation.isMainStation)

If you're wondering what's wrong with that line, the answer is: nothing. It works fine... now. But it didn't look that way when I first started. In fact, it looked like a horse of a whole different color. I spent the better part of the afternoon hammering away until Oolite finally stopped saying, "and just what the hell is this??" Sheesh

My hat's off to all you guys skilled in the ancient, mystical art of JavaScripting.

[Thargoid]

And that line would be improved by actually checking you were docked first, or else it'll throw an error

I wouldn't worry about it, after the first couple of thousand of them you start to get the hang of things...

[Diziet Sma]

And then comes the testing..

Getting Oolite to stop complaining about your code is one thing. Getting the code to actually do what you wanted is quite another.

Unfortunately, despite millions of coders wishing for it, and decades of waiting, and dozens of programming languages, nobody has yet managed to create a do (what.I.mean) function...

[Massively Locked]

Thargoid-

Yes, actually I had included a check-for-docked. Here's my script in its full glory:

Code: Select all

this.shipDockedWithStation = function(station)
	{if (player.ship.dockedStation.isMainStation)
		mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
		}

While it may seem ridiculously basic to a lot of you, it took me HOURS (no joke) to put that together.

About what you wrote regarding the learning curve- truer words never spoken.

Diziet-

I totally hear you on that, but I'm nowhere near that stage yet. I just want to write a few lines of code without it blowing up like, well, like my ship just did a little while ago (God damned Fer-de-Lance assassin- he's off my Christmas list next year).

[Diziet Sma]

Every step along the way helps.. "ah-hah!" moments come along and the pennies start to drop.. and like everything else in life, we learn more from our mistakes than the things we get right. Before long, you'll be surprising yourself. Even the Devs (Praise Be Unto Them) once stood where you are now..

[Wildeblood]

Here's my script in its full glory:

Code: Select all

this.shipDockedWithStation = function(station)
	{if (player.ship.dockedStation.isMainStation)
		mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
		}

You overlooked something:-

Code: Select all

this.shipDockedWithStation = function(station) // <----------
	{if (station.isMainStation) // <-----------
		mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
		}

[submersible]

You overlooked something:-

Code: Select all

this.shipDockedWithStation = function(station) // <----------
	{if (station.isMainStation) // <-----------
		mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
		}

The first one is cuddled on the next line , the inner if is implicit single statement. Reformatted...

Code: Select all

this.ShipDockedWithStation = function(station)
{
    if (station.isMainStation)
        mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
}

For clarity - I would always go for braces despite there being only a single statement conditional on station.isMainStation

Code: Select all

this.ShipDockedWithStation = function(station)
{
    if (station.isMainStation)
    {
        mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
    }
}

[Wildeblood]

You overlooked something:-

Code: Select all

this.shipDockedWithStation = function(station) // <----------
	{if (station.isMainStation) // <-----------
		mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
		}

The first one is cuddled on the next line , the inner if is implicit single statement.

Huh? I was pointing out the station reference going to waste, not commenting on the formatting.

[Eric Walch]

There is just one flaw left in the code:

Code: Select all

this.ShipDockedWithStation = function(station)
{
    if (station.isMainStation)
    {
        mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
    }
}

It does not check if another script already presented a mission screen. In that case this oxp would overwrite that screen. So, it needs an additional check:

Code: Select all

this.ShipDockedWithStation = function(station)
{
    if (station.isMainStation && guiScreen != "GUI_SCREEN_MISSION")
    {
        mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
    }
}

To avoid this type of problems, it is strongly advised to use the missionScreenOpportunity to display missionscreens. When your script gets a missionScreenOpportunity you can be sure no other script is using the missionscreen.

[submersible]

Huh? I was pointing out the station reference going to waste, not commenting on the formatting.

Ah , totally mis-interpreted your added comments. But why is 'station' going to waste?

[Massively Locked]

Wildeblood- that's great! I did wonder what 'function(station)' was all about since I didn't use it in my subsequent lines. I'm also pretty sure that I tried 'station.isMainStation', but it didn't work probably because my first line originally ended with "function()". It's all so clear now.

submersible- good advice on braces & formatting. The one thing I know about parens, braces and brackets is always close what you open. Beyond that, I'm hazy on when and where they're best used. As for why the station ref was going to waste, I just found another way to get the 'If' condition working and went with that.

Eric- I'm definitely gonna add and study your addition. After I finally got my script working, I did come across 'missionScreenOpportunity', and it seemed to be what I need to use. That will take me some time figure out.

It's funny that after I thought I was finished with my little script, I wasn't actually finished with it. I wasn't expecting the tips and advice, and it's much appreciated. As Diziet said above, you really do learn more when things go wrong than when they go right.

[Smivs]

You also learn a lot from the good folks around this board The wealth of knowledge here is impressive, and always so freely given.
Yes, it is a steep curve, and learning js can be frustrating as much as rewarding, but it's definitely worth the effort, and the possibilities js gives us seem endless. The biggest limit is in the imagination of the coder - if you can imagine something, one way or another you can probably make it work.

[Eric Walch]

It's funny that after I thought I was finished with my little script, I wasn't actually finished with it.

Most of the writing time is not going into what you want to do with the script, but in handling all the exceptions were your script should do nothing.....

[Tricky]

Also read other OXP scripts for examples on how to handle things.

@Eric: Reminds me of some code I wrote, here's the comment...

Code: Select all

/* START OF CODE THAT SHOULD NEVER BE REACHED.
 * This is here purely for error checking sake.
 * If the base is destroyed it will set the patrol ships fully launched variable,
 * therefore this code block shouldn't be reached.
 */

In theory it could be executed so I coded it to handle it.

Edit: Comment your code so that when you come back to it after a few months you know what you were trying to do.

[Commander McLane]

There is just one flaw left in the code:

Code: Select all

this.ShipDockedWithStation = function(station)
{
    if (station.isMainStation)
    {
        mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
    }
}

It does not check if another script already presented a mission screen. In that case this oxp would overwrite that screen. So, it needs an additional check:

Code: Select all

this.ShipDockedWithStation = function(station)
{
    if (station.isMainStation && guiScreen != "GUI_SCREEN_MISSION")
    {
        mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
    }
}

To avoid this type of problems, it is strongly advised to use the missionScreenOpportunity to display missionscreens. When your script gets a missionScreenOpportunity you can be sure no other script is using the missionscreen.

Which means that the better way of doing it would be:

Code: Select all

this.missionScreenOpportunity = function()
{
    if (player.ship.isDocked && player.ship.dockedStation.isMainStation)
    {
        mission.runScreen({title: "Automated GalCop Station Greeting", message: "Welcome, Commander"});
    }
}

@Massively Locked: two comments:

1) You notice that player.ship.dockedStation is back again. That's because the missionScreenOpportunity-handler does not have the handy station-parameter that can be used inside the function.

And 2) Thargoid's first comment is accommodated by adding a first check for whether the player is actually still docked at this point. This may seem superfluous at first glance, but experience has shown that it isn't, because there could be another script active that could have force-launched the player in-between calling the missionScreenOpportunity-handler and checking your condition.

(And by the way: Eric in his code transports an error introduced by submersible, so there were actually two flaws. Can you spot it? Answer in the fine print.)





All reserved names in JS begin with a lowercase. Thus ShipDockedWithStation must be [color=#FF0000]s[/color]hipDockedWithStation