OoliteProject / oolite

The main Oolite repository.
https://www.oolite.space
567 stars 73 forks source link

Mission.allowInterupt = true unstable or unsafe when using framecallbacks in 1.84 note sure about latest #309

Closed DTHP closed 6 years ago

DTHP commented 6 years ago

Using mission.allowInterupt = true that allows the functionality during mission screens of the F keys is unsafe when using framecallbacks to animate mission.displayModel

The frame callbacks can when mission.allowInterupt = true no more be removed via removeFramecallback(ID).

the OXP is not in a state to be transistioned to latest build. But it is likely you have the same problem in latest build.

isValidFrameCallback can no more identify it as a prober framecallback and just bruteforce trying to remove it yields the same result. The frame callback is predictable not stopped and invoking more instances will eventually end the debugger window.

example

yadda yadda.. var parameters = new Object(); parameters.allowInterrupt = false; parameters.model = "pirate"; parameters.title = "something about PI"; parameters.message = "round and round and round."; mission.runScreen(parameters, this.callback);

this.HandleChoices = addFrameCallback(this.FramecallbackMChoices.bind(this)); }

this.FramecallbackMChoices = function(delta) { log("once upon a time")

//organize
mission.displayModel.position = [50, 75, 300]
mission.displayModel.orientation = [0,0,0,1]
if(isValidFrameCallback(this.HandleChoices))
{
    log("removed handle")
    removeFrameCallback(this.HandleChoices);
    this.checker = addFrameCallback(this.SysDataView.bind(this));
}

}

this.SysDataView = function(delta) { if(guiScreen != "GUI_SCREEN_MISSION") { log("removing callBack via no GUi mission") if(isValidFrameCallback(this.checker)) removeFrameCallback(this.checker) return }

//else slightly rotate model
if(delta > 0)
{
    q = new Quaternion(mission.displayModel.orientation)
    mission.displayModel.orientation = q.rotate([0,1,0],(Math.PI/180)*delta*10)
}

}

this.callBack = function() { //do something about choices }

CmdrSvengali commented 6 years ago

If you are doing it this way it's likely that you are overwriting your own tracking IDs, resulting in lose callbacks. Specially if multiple screens are shown you'll never remove this.checker.

A simple approach could look like:

this.missionScreenOpportunity = function(){
    if(this.$MD) return; // Check your conditions to show your screen.
    mission.runScreen( {allowInterrupt:true, model:"pirate", spinModel:false} );
    var md = mission.displayModel;
    if(md && md.isValid){ // Do only if model is displayed. Could be disallowed via condition_script.
        this.$MD = {KEY:md.dataKey, ID:md.entityPersonality}; // Store your models identification.
        md.position = [50, 75, 300];
        md.orientation = [0,0,0,1];
        this.$FCB = addFrameCallback(this._funnyFCB.bind(this));
    }
};
this._funnyFCB = function(delta){
    var md = mission.displayModel;
    // Cleanup if no displayModel, or neither dataKey nor entityPersonality do match.
    if(!md || !md.isValid || md.dataKey !== this.$MD.KEY || md.entityPersonality !== this.$MD.ID){
        removeFrameCallback(this.$FCB);
        return;
    }
    if(delta) md.orientation = md.orientation.rotate([0,1,0],(Math.PI/180)*delta*10);
};
// And at some point clean this.$MD to show the next screen, e.g. on launching.

Close?

AnotherCommander commented 6 years ago

Yes, I think we can close.

DTHP commented 6 years ago

I fixed it in sort of the sameway. The script of the displayModel (it is only used as a displayModel) takes care of rotating it and is deleted upon the removal of the display model.