Updating the OSE deep space pirates script

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

Moderators: another_commander, winston, Getafix

pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Updating the OSE deep space pirates script

Post by pmw57 »

deep_space_pirates.js for OSE is now updated.

Running the code through jslint.com showed that the script had become wild and wooly. Several parts of it have been refactored to bring things back under control.

The addPirates method is now called managePirates, which in turn calls addPirates and addAsteroids

The pirate ships themselves are also obtained now using an object oriented technique, which is more in keeping towards the OO nature of the game.

Before:

Code: Select all

	allPirates = system.shipsWithPrimaryRole("hardpirate", player.ship, 60E3);
	function newPirates(entity) {return ((clock.absoluteSeconds - entity.spawnTime) < 5)}; // remove old pirates from list.
	pirates = allPirates.filter(newPirates);
After:

Code: Select all

	pirates = system
		.shipsWithPrimaryRole("hardpirate", player.ship, 60E3)
		.filter(function newPirates(entity) {
			// remove old pirates from list.
			return (clock.absoluteSeconds - entity.spawnTime) < 5;
		}
	);
You will also see such techniques used extensively in OO libraries such as jQuery.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
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

Post by Kaks »

While you're there, calling the filter function newPirates is a bit superfluous / redundant now... an anonymous function should work just as well! :)
Hey, free OXPs: farsun v1.05 & tty v0.5! :0)
User avatar
Thargoid
Thargoid
Thargoid
Posts: 5525
Joined: Thu Jun 12, 2008 6:55 pm

Post by Thargoid »

And this new standardised code is supposed to be readable and understandable by people wanting to learn JS?

If I saw the "after" one I'd give up and walk away.
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

Kaks wrote:
While you're there, calling the filter function newPirates is a bit superfluous / redundant now... an anonymous function should work just as well! :)
The benefit of using a named function expression in this situation outweighs that of an anonymous function expression. The named function expression is one of those little-known abilities of javascript which make understanding the code a lot easier to achieve.

If you want to find out more, you can read up about it at places like http://yura.thinkweb2.com/named-functio ... named-expr

Note: My choice for using a named function expression was not because it's fashionable. It is just the best tool to use in that particular situation in the code.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

Thargoid wrote:
And this new standardised code is supposed to be readable and understandable by people wanting to learn JS?

If I saw the "after" one I'd give up and walk away.
I agree, the object oriented one can be upsetting if you come from procedural background.

A possible compromise could be something like this:

Code: Select all

	allPirates = system.shipsWithPrimaryRole("hardpirate", player.ship, 60E3); 
	pirates = allPirates.filter(function newPirates(entity) { 
		// remove old pirates from list. 
		return (clock.absoluteSeconds - entity.spawnTime) < 5; 
	});
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Post by Eric Walch »

I'll give it a look. But I said it many times before: Why do you not work on the recent versions but only on old stuff. The whole stuff I added for 1.73 about shipGoups is missing. Also In this old version I had problems with my escorts because 1.73 deals it a bit different than than before, so I had to re-code parts to let it work with 1.73.
(With 1.72 escorts are not escorting their mother yet. The array of the escort property of their mother is empty. To be able to add custom scripting to the escort I had to use the "acceptedAsEscort" handler to add this code. With he new groups, escorts are immediately part of the group and the acceptedAsEscort only fires for the second mother then loosing the first.)

And than there were problems because there is still a bug in 1.73 with groups that will be fixed in 1.74. Writing my additions I did it in a way it will work both in current 1.73 and future 1.74 were this rare group bug is corrected.

For me this code is now a nice example code but not suitable for replacement as I am likely to readd bugs I already fixed some time ago. And because the structure is different now it would take to much time to figure out what is a real change or only a struckture change.
Last edited by Eric Walch on Fri Oct 16, 2009 1:48 pm, edited 1 time in total.
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

Post by Kaks »

pmw57, thank you for your concern about improving my knowledge of js.

While on the subject, in another thread you warned people against the use of let, but you might not have been aware of all the nuances of the language as implemented within Oolite: the use of let instead of var is explained here:

https://developer.mozilla.org/en/New_in ... e_with_let

Glad to be of service! :)


And by the way, jQuery and similar try to use as compact a code as possible to minimise bandwidth usage. To jQuery-fy Oolite's scripts is quite unnecessary, since there's no bandwidth to worry about.


To my mind, defining the filter function first, and then applying it to the array is much easier to digest for people relatively new to the language.
What Thargoid just said makes a lot of sense to me, especially if we want to encourage people to write their own js scripts.

On the other hand, if you want the code as streamlined as possible, which is what it looks like to me, keeping it as a named function doesn't really seem to make any difference to readability.
After all you do have a comment inside the function already saying what it's supposed to do...

PS & edit: it does look as if you are taking OSE's codebase as the definitive version of scripts. In quite a few cases, sadly, the original developers did stop developing their OXPs once RS / OSE 'took over', but in a few other instances, what you see are just copies of old code that has already been updated and bugfixed in the original OXPs.

Before updating an 'OSE' script you might want to double check the actual oxp that script was taken from in the first place, just to see if OSE has missed any updates to the original. That way it might save you a lot of headaches...

Hope this helps!
Last edited by Kaks on Fri Oct 16, 2009 1:30 pm, edited 1 time in total.
Hey, free OXPs: farsun v1.05 & tty v0.5! :0)
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

Eric Walch wrote:
I'll give it a look. But I said it many times before: Why do you not work on the recent versions but only on old stuff. The whole stuff I added for 1.73 about shipGoups is missing. Also In this old version I had problems with my escorts because 1.73 deals it a bit different than than before, so I had to re-code parts to let it work with 1.73.

And than there were problems because there is still a bug in 1.73 with groups that will be fixed in 1.74. Writing my additions I did it in a way it will work both in current 1.73 and future 1.74 were this rare group bug is corrected.

For me this code is now a nice example code but not suitable for replacement as I am likely to readd bugs I already fixed some time ago. And because the structure is different now it would take to much time to figure out what is a real change or only a struckture change.
I'll come back to this tomorrow. I'm currently working from the code that lestradae has put together for OSE. As most OXPs consist of more than just a script, it is up to L to incorporate your latest version into the rest of OSE. If he decides to do that, and I sincerely hope that he does, I'm happy to revisit the latest version.

To help clarify the structural change that occurred, the parts that add pirates and that add asteroids were pulled out to separate methods.

Before:
check
- addPirates
- do many things

After
check
- managePirates (was addPirates)
- addPirates
- addAsteroids

I think that you'll find quite a benefit in doing that.
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
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

Post by Kaks »

As noted in another thread, it would make a lot of sense to have the OSE-ified scripts to at least start with OSE- so at least we know if we're talking about the original or not.
Hey, free OXPs: farsun v1.05 & tty v0.5! :0)
pmw57
---- E L I T E ----
---- E L I T E ----
Posts: 389
Joined: Sat Sep 26, 2009 2:14 pm
Location: Christchurch, New Zealand

Post by pmw57 »

Kaks wrote:
While on the subject, in another thread you warned people against the use of let, but you might not have been aware of all the nuances of the language as implemented within Oolite: the use of let instead of var is explained here:

https://developer.mozilla.org/en/New_in ... e_with_let
That's excellent to know that the Oolite scripting engine supports JavaScript 1.7. Would you happen to know how far the support in Oolite goes?
Kaks wrote:
And by the way, jQuery and similar try to use as compact a code as possible to minimise bandwidth usage. To jQuery-fy Oolite's scripts is quite unnecessary, since there's no bandwidth to worry about.
Without the use of the let keyword, such OO techniques are a useful tool to reduce the number of variables being dealt with. Knowing now that Oolite is capable of fully supporting block-level let statements allows for a greater flexibility. Thanks!
A trumble a day keeps the doctor away, and the tax man;
even the Grim Reaper keeps his distance.
-- Paul Wilkins
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

Post by Kaks »

pmw57 wrote:
That's excellent to know that the Oolite scripting engine supports JavaScript 1.7. Would you happen to know how far the support in Oolite goes?
We use the whole of the mozilla spidermonkey engine: so I'd say it goes quite far... :)
Hey, free OXPs: farsun v1.05 & tty v0.5! :0)
User avatar
Eric Walch
Slightly Grand Rear Admiral
Slightly Grand Rear Admiral
Posts: 5536
Joined: Sat Jun 16, 2007 3:48 pm
Location: Netherlands

Post by Eric Walch »

Kaks wrote:
pmw57 wrote:
That's excellent to know that the Oolite scripting engine supports JavaScript 1.7. Would you happen to know how far the support in Oolite goes?
We use the whole of the mozilla spidermonkey engine: so I'd say it goes quite far... :)
But the information about let is quite new to me. I think two years ago Ahruman warned us about the use of let as it might be deprecated or slightly changed in function. In the past let was explained on the mozilla pages but a good year ago the let references were greyed out on their site.
This page is newly edited so it probably means let stays in? In Oolite it always has worked since oolite version 1.70.
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

Post by Kaks »

Well, ecmascript 4 - which was the javascript proposed standard that described let amongst other things - collapsed in a heap of rubble when m$ & yahoo decided they didn't like it. At the moment there are 2 proposed standards, which still need to be finalised: ecmascript 3.1 aka Harmony, which everybody apparently is going to be happy about, and ecmascript 5, which again everybody should agree upon at some point in the future.

Thanks to the ecmascript 4 wobbly no-one is really sure what the final new javascript standard is really going to look like, so a bit of caution is very much understandable.

However, mozilla's js 1.7 does support quite a lot of the almost-standard-but-not-yet extensions to the language that were agreed upon for ecmascript 4. To the best of my knowledge, they have no plans to remove any of the new stuff in the future either.
Hey, free OXPs: farsun v1.05 & tty v0.5! :0)
User avatar
Lestradae
---- E L I T E ----
---- E L I T E ----
Posts: 3095
Joined: Tue Apr 17, 2007 10:30 pm
Location: Vienna, Austria

..

Post by Lestradae »

As I already said in another thread, I will "OSE-ify" the OSE scripts as it seems they are starting to diverge on the original oxps they stem from.

Perhaps the deeper question might be if this is a good idea at all, I will have to think about that. I am actually seeing myself as putting this stuff together into a big bundle, but I don't (and can't) change a lot, I add additional functionalities here and there and use synergies, but it mustn't go overboard in order for me to be able to continue updating the OSE project.

I am btw quite surprised that the scripts I use are not the newest. To the day before pmw57 and Screet started redoing the original java scripts I browsed the boards after every scrap of change and immediately altered OSE with any oxp changes I observed. That means that if I don't get all the changes who make a point of it, new users won't get them at all ...

Which is something perhaps also the veteran oxp'ers here might think through, too, in its consequences. It's cool if its updated weekly but no one knows about it.

Perhaps the scripts business should be done in a much more cooperative way than this first attempt?

pmw57, Screet, Kaks, Eric, what are your opinions on this? The common aim I assume is that players get as updated and well-working oxps as possible, if it is the original or OSE or whatever combination?

:?

L
Screet
---- E L I T E ----
---- E L I T E ----
Posts: 1883
Joined: Wed Dec 10, 2008 3:02 am
Location: Bremen, Germany

Re: ..

Post by Screet »

Lestradae wrote:
pmw57, Screet, Kaks, Eric, what are your opinions on this? The common aim I assume is that players get as updated and well-working oxps as possible, if it is the original or OSE or whatever combination?
I think it might take some time with "find in files" for each scripts name to find all the places that need to be changed, but renaming them to something like ose_<scriptname> should be fine and might also add some safety is someone has a mixed installation.

Concerning the trouble to find the most recent version: Well, the wiki is back, thus I'd first look there.

However, when I was gone for some months, came back and did dl OSE, I noticed that OSE did come sometimes with newer versions than I had using that way. Looks like it's sometimes really easy to miss an update.

Screet
Post Reply