this.missionScreenEnded mini-bug

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

Moderators: winston, another_commander, Getafix

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

this.missionScreenEnded mini-bug

Post by Kaks »

If you launch directly from a mission screen, without pressing space, this.missionScreenEnded doesn't fire.

I had

Code: Select all

this.missionScreenEnded=function(){
	mission.clearMissionScreen();
}
which would clear the mission background only if I pressed space.
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Post by JensAyton »

Fixed.

In future, missionScreenEnded will be called before shipWillLaunchFromStation if a mission screen is in effect. Thus, code like this should do the Right Thing for 1.70 and onwards:

Code: Select all

this.doingMissionScreen = false // Set to true when starting a mission screen

this.shipWillLaunchFromStation = function(station)
{
    if (this.doingMissionScreen)  this.missionScreenEnded() // Should only happen in 1.70
}

this.missionScreenEnded = function()
{
    if (this.doingMissionScreen)
    {
        mission.clearMissionScreen()
        // Any post-mission handling
        this.doingMissionScreen = false
    }
}
Note: missionScreenEnded is sent to all world scripts (and the player ship script) when a mission screen end, so you should only be doing things in it if you think you’re currently in a mission screen. Look at oolite-trumbles-mission.js for an example.
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 »

Ahruman wrote:
Note: missionScreenEnded is sent to all world scripts (and the player ship script) when a mission screen end, so you should only be doing things in it if you think you’re currently in a mission screen. Look at oolite-trumbles-mission.js for an example.
And into the Java translations of UPS were I copied this. But I still think one needs a common variable among scripts like the "mission_offering" I am consequently using, to tell that the script wants to set up a second mission screen. Without this variable and other scripts that respect this variable status, you can get the situation that script 1 sets up his 1st screen, script 2 shows the second screen and script 1 shows his second screen. Mission screens are interfering with each other. But not really clashing in the way you loose information, just the display order will not be correct. It will be rare however as multiple screen offers are rare and there must be a second script also setting up a mission screen.

I now added a parcel mission JS translation and uploaded the whole as version 1.3.1. Two mission series don't use the tickle event anymore and a 3th mission series uses it only for one single in-flight check. :UPS-Courier
------
EDIT
On testing I found an other related "bug". Could be present since 1.65 or older but I tested it with 1.70. I was just testing what would happen with multiple briefings.

I set up my mission variables so I would get an offer on the next docking. Then I went for pirate shooting until I scooped up an escape pod. I just wondered witch message would come first. But on docking (briefing shown with: this.shipDockedWithStation) I only got my missionbriefing. Not the message of the reward for a captured criminal.

In legacy scripting I showed my briefing when "GUI_SCREEN_STATUS". Then I always got the captured pilot message first. It could be that when I show my briefing on "GUI_SCREEN_MAIN", the captures pilot messages would also be gone.

So I designed a test ship. "has_escape_pad = 5" and with a scripted pilot. On shooting that ship he released 5 escape pods of witch one was the scripted one. I scooped them all.

On docking I first got the message from the scripted pilot. (legacy script) The message scheduled for docking was skipped at that moment as the missionscreen was occupied. (JS) I added also an offeringcheck in this. missionScreenEnded, so on hitting space with the first screen, I got my missionbriefing. (JS) But the captured pilot message never came. But I noticed I did get the bounty's for those pilots. Also the 5 slaves from the inventory were removed.

Writing this I don't know if it is a bug, or just deliberate to avoid getting to much messages for the player as the captured pilot messages are not that important as long as the bookkeeping is correct.
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 »

That the eventhandler this.missionScreenEnded runs before of after the launching hanhdlers does not matter. It leads not to a bug.
However, clearing the missionscreen on ending is!!

Code: Select all

this.missionScreenEnded = function()
{
    if (this.doingMissionScreen)
    {
        mission.clearMissionScreen()
		mission.setBackgroundImage('none');
		//LogWithClass('script.'+this.name , 'screen ended!!');
        // Any post-mission handling
        this.doingMissionScreen = false
    }
}
When setting up a missionscreen without choices you can setup everything together with the screen.

when you have for example a trumble salesman offering you his merchandise and you make your choices. the mission screen ends. Whenever a script receives an missionScreenEnded event he does not know who did set up the screen. When he just clears the screen he also clears the choices of the other oxp.
When that oxp receives the missionScreenEnded as second he is screwed up.

When I look at the internal trumble message I see that Auruman was still a little friendly.: He only sets mission_trubmles = "BUY_ME" when he is showing the mission screen. So when the screen ended event comes up, the answer can only be TRUMBLE_YES or TRUMBLE_NO. In his last else he comments "than it is sombodies else screen" Wrong: some other script messed up your screen. This else condition should lead to setting up an error message. (I'll do in my next UPS release)
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Post by JensAyton »

It could be someone else’s mission screen. For instance, if someone left the station while the trumble mission screen was up, prior to revision 1339. :-)
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 »

It could be someone else’s mission screen. For instance, if someone left the station while the trumble mission screen was up, prior to revision 1339. :-)
It could. I already thought of that. I was already one step further as I am resetting any mission choices in UPS on launch. I added that long ago as I noticed that several other missions didn't reset this properly and forcing my script not to offer anything as i could not figure out If there would come a reaction. So when you clear the choices on launch, it can't be some others screen.
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 »

Ahruman,

better remove the new function resetMissionChoice() from the new introduced package clearMissionScreen(). It will only lead to confusion in future as this has no direct link with the screen itself.

Even stronger:

When I used to set up a message with choices I always checked first If there was one active choice from an other script. If yes I skipped the message to not overwrite his choices.

But when I set up a message without choices I don't look at an active choice. I just clear the screen and put up my message. Because I don't reset the choice an other script can still react on its choiceKey. With my current UPS JS release I just realised that by using clearMissionScreen() I also erase his choises and I have to use the three clearing functions individually (clear: music, ship and background). This means have all 4 in one package makes things even complicated as when not offering choices you must to avoid one of the four. (or just skip the whole message)

Removing it will be no big deal as there are no published scripts using the function right now (except my preliminary UPS version and some that are in devellopment but have active scripters that can change the scrip directly before the first release)

clearMissionScreen() is not even mentioned on the wiki.

EDIT:
On second thought: Make the choices local to the script that called them. That way no other script can delete them. That way it is only the wrong script itself that get bugged. By cleaning up to much.
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Post by JensAyton »

Eric Walch wrote:
EDIT:
On second thought: Make the choices local to the script that called them
I don’t see a clean way to do this in the current structure. A better way of handling mission screens (creating Mission objects, customizing them and then showing them atomically, with no means of cross-contamination between screens) is one of those would-be-good-in-future things.

In the meantime, I’ve removed resetMissionChoice from clearMissionScreen on the basis that you clearly have a better grasp of writing mission screens than I do. :-)
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 »

In the meantime, I’ve removed resetMissionChoice from clearMissionScreen on the basis that you clearly have a better grasp of writing mission screens than I do. :-)
Thanks, I saw future bug occurring with misuse of this function as the name is misleading when it does more than just clearing things of the missionscreen. In my newest UPS I do the missionscreens in a special function. That is the most easiest way and you don't have to do any clearing every time. The function takes care of that.
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 »

It is OK to generate a "missionScreenEnded" event when launching from within a missionscreen without hitting the right key. But in general you don't need it. At least I can't see when you need it. It even adds a need to always check if the player is still docked before further processing the event otherwise you end in setting up a missionscreen in space. But leave it in as someone could make use of it once.

The example of kaks were he thought he needed it was a bad one. He does not need the event to clean up the remainders of his missionscreen. Clearing a screen has only effect on the next screen to display, so cleaning up can already be done directly after setting it up as shown in the next function. The function itself I copied from Kaks his work with modifications to make it a more general function.

Code: Select all

this.missionScreen=function(messageKey, backGround, choiceKey, myMusic, myShipModel)
{
        mission.showShipModel(myShipModel);mission.setMusic(myMusic)
	mission.setBackgroundImage(backGround);
	mission.showMissionScreen();
	mission.addMessageTextKey(messageKey);
        if (choiceKey != null) mission.setChoicesKey(choiceKey)
	mission.clearMissionScreen();
}
After this you only need one command to show a missionscreen and do all the settings and clearings:

this.missionScreen("ups_docs_1st_offer", "UPS.png", "ups_docs_accepted_yesno", null, null)

this.missionScreen("ups_docs_1st_delivery", "UPS.png", null, null, null)

For the parameters you don't need, you fill in the "null" parameter and this part will be cleared in the function. After setting up you can already clear everything at the end of the same function. Currently, in 1.70, you can not use the "clearMissionScreen()" as it will also clear missionchoices that an other OXP could still need to process. So in 1.70 you need to use the 3 individual clearings. For 1.71 the choice clearing will be removed from the package and "mission.setMusic(myMusic)" will be working.

For 1.70 still use:

Code: Select all

this.missionScreen=function(messageKey, backGround, choiceKey, myMusic, myShipModel)
{
        mission.showShipModel(myShipModel);mission.setMusic(myMusic)
	mission.setBackgroundImage(backGround);
	mission.showMissionScreen();
	mission.addMessageTextKey(messageKey);
        if (choiceKey != null) mission.setChoicesKey(choiceKey)
	mission.showShipModel();mission.setMusic()
	mission.setBackgroundImage();
}
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 »

Hey, did I hear my name? :D

Thanks about the extra work on this.missionScreen. There's just two points I'd like to add:

- instead of mission.setMusic you can use

Code: Select all

player.call("playSound:", 'yoursound.ogg'); 
to make the sound work on a windows / linux computer in Oolite 1.70. The ogg file needs to be inside the Sound directory instead of the Music one. That way everybody hears the mission sound!

- javascript automatically assigns the value null to all the function's parameters, then replaces those nulls with the values you provide when you call that function. You don't need to add the extra null parameters at the end. To test if a function variable has been assigned a value: if (variable) is exactly the same as if (variable != null) in javascript.

With those two points in mind, the generic missionScreen function could be rewritten like this:

Code: Select all

this.missionScreen=function(messageKey, backGround, choiceKey, myMusic, myShipModel)
{
    mission.showShipModel(myShipModel);
    player.call('playSound:', myMusic); //found in the Sound directory, not Music
    mission.setBackgroundImage(backGround);
    mission.showMissionScreen();
    mission.addMessageTextKey(messageKey);
    if (choiceKey) mission.setChoicesKey(choiceKey);
    mission.showShipModel();
    mission.setBackgroundImage();
}
and the two examples of calling the function can be written as:

Code: Select all

this.missionScreen("ups_docs_1st_offer", "UPS.png", "ups_docs_accepted_yesno") 

//and

this.missionScreen("ups_docs_1st_delivery", "UPS.png") 

The beauty of javascript is, of course, its flexibility!

One thing that doesn't seem to work for me, on a windows pc, is the

Code: Select all

 mission.setBackgroundImage();
bit. What I wanted to do was to clear the background image if the player pressed 1 to leave the station in the middle of a briefing. If I press 1 and - once in flight - press 6, the mission background is displayed with the local map. I'm still trying to figure out a way to solve that problem. Who knows, it might be sorted out for 1.71! :)

Cheers,

Kaks
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Post by JensAyton »

Kaks wrote:
Hey, did I hear my name? :D

Thanks about the extra work on this.missionScreen. There's just two points I'd like to add:

- instead of mission.setMusic you can use

Code: Select all

player.call("playSound:", 'yoursound.ogg'); 
to make the sound work on a windows / linux computer in Oolite 1.70. The ogg file needs to be inside the Sound directory instead of the Music one. That way everybody hears the mission sound!
Music is supposed to work cross-platform in the trunk, though. If it doesn’t, I’d like to hear about it. :-)
One thing that doesn't seem to work for me, on a windows pc, is the

Code: Select all

 mission.setBackgroundImage();
bit. What I wanted to do was to clear the background image if the player pressed 1 to leave the station in the middle of a briefing. If I press 1 and - once in flight - press 6, the mission background is displayed with the local map. I'm still trying to figure out a way to solve that problem. Who knows, it might be sorted out for 1.71! :)
Possibly fixed in r1370, please test.
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 »

just tested with svn r1370 and yes,those two mission related bugs are fixed!

mission.setMusic now works on windows.

exiting the mission screen by leaving the station: the background image is cleared from the map, planet info, etc. screens.

PS. I'd remove the extra

Code: Select all

    mission.setBackgroundImage(); 
at the end of the this.missionScreen function.

If I understand it correctly, that bit of javascript tells r1370 to remove the background image immediately after calling mission.showMissionScreen.

Cheers,

Kaks.
User avatar
JensAyton
Grand Admiral Emeritus
Grand Admiral Emeritus
Posts: 6657
Joined: Sat Apr 02, 2005 2:43 pm
Location: Sweden
Contact:

Post by JensAyton »

Kaks wrote:
mission.setMusic now works on windows.
In that case, I shall entertain the hope that Sound.playMusic() does, too. :-)
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 »

Confirmed!

Code: Select all

Sound.playMusic('musicfile.ogg');
works in windows. :)
Post Reply