0 votes
by (420 points)
edited by

I'm seeing a strange scenario when I load a saved game -- an object that is created without issue when I never load is apparently not getting created before it is accessed.

The configuration is as follows:

-variable State.variables.combatArena, which is an instance of the State.variables.Combat class is defined as undefined in story javascript.

-there are three key passages: an encounter passage with some flavor text, a boss fight passage which runs a script to set up the combat, and several combat UI passages:

Encounter_Puck_Viral_Violence

"My name is Puck, knight captain and exalted archmage of her highness Holly, Wild Goddess of All Creation!  Feeble creatures, [[die!->Puck_Bossfight]]"

Puck_Bossfight

/*set combat params and forward (valiantly) to Combat!*/
<<script>>

var characters = State.variables.characters;
var monster_party = [characters["puck"]];
console.log("about to create combat arena object");
State.variables.combatArena = new State.variables.Combat(State.variables.party,monster_party,passage())
State.variables.combatArena.turnOwner = "player"
<</script>>
<<goto [[Combat]]>>

Combat

<<include "CombatLog">>
<<include "PlayerCombatDisplay">>
<<include "PlayerPartyStats">>

CombatLog

<div id="combatLog">
$combatArena.combatLogContent
</div>

PlayerCombatDisplay

<div id="playerCombatWindow">
<<if $combatArena.turnGroup is "player">>
...

-the problem I'm seeing after I hit the link text in Encounter_Puck_Viral_Violence  to start the combat is that  CombatLog shows the literal text $combatArena.combatLogContent and the PlayerCombatDisplay shows Error: <<if>>: bad conditional expression in <<if>> clause: Cannot read property 'turnGroup' of undefined.  

This only occurs if I load a save at some point prior to this point (running through the story and combat in one sitting works fine).  It seems like the State.variables.combatArena object is not getting set in Puck_Bossfight in this case -- any idea why that might be?  The console.log() message inside the <<script>> tag is not printed, so it looks like that tag is getting skipped entirely for some reason.

UPDATE: an alert at the same position as the console.log() call does fire, so the script tag is running; the underlying object is just undefined after load.

1 Answer

+1 vote
by (159k points)
selected by
 
Best answer

You don't actually show the definition of the State.variables.Combat object value, so based on the way you are calling it I have to assume that it's either:
1. a constructor function that returns a custom complex object (possible with prototype based methods)
2. a ES6 class with a constructor.
3. something just as complex as the other two options.

You also don't state exactly when the State.variables.Combat object value is first initialised within your story, if it's not done during the startup process then that story variable may not actually exist within any particular Save. 

SugarCube uses JSON to serialise / deserialise the objects stored within the story variables of a Save, and you need to tell JSON how to reconstuct complex object types or classes. The answer to the Objects that use Prototypes and how to serialize them properly to enable saving and loading question explains about doing this.

by (420 points)

very interesting!  In the linked answer, TheMadExile gives the following toJSON() example:

ContactInfo.prototype.toJSON = function () {
	// Return a code string that will create a new instance
	// containing our current data.
	return JSON.reviveWrapper('new ContactInfo($ReviveData$)', this);
};

what is ReviveData in this case and where would it be coming from?  Is it a story variable holding the current ContactInfo instance?

My Combat object is defined in the story javascript as:

State.variables.Combat = function Combat(playerParty, enemyParty, destPassageName) {
    this.PlayerTurnState = Object.freeze({
        selectTarget: 1,
        selectAbility: 2,
        displayResults: 3,
        selectEnemyTarget: 4,
        selectAllyTarget: 5
    });
    this.EnemyTurnState = Object.freeze({
        runAI: 1,
        displayResults: 2
    });
    this.CombatResultEnum = Object.freeze({
        pending: 0,
        playerVictory: 1,
        enemyVictory: 2
    });

    // metadata about what state the player's turn is in.  This is necessary to help route the UI around the various actions a human player will need to input
    this.playerTurnState = this.PlayerTurnState.selectAbility;
    this.enemyTurnState = this.EnemyTurnState.runAI;
    this.playerParty = playerParty;
    this.enemyParty = enemyParty;
    this.destPassageName = destPassageName;
    this.combatResult = this.CombatResultEnum.pending;

    // tracks the currently selected player ability
    this.currentSelectedAbility = new State.variables.Ability("unset");

    // tracks the turn as either player or enemy group
    this.turnGroup = "player";

    // tracks the actual character id whose turn it is
    this.turnOwner = "player";

    // tracks the currently selected ability for the current turn owner
    this.abilityName = "";

    // tracks the target of the current ability
    this.currentTargetCharacter = new State.variables.Character("unset");

    // the text feedback to the user re: the state of combat
    this.combatLogContent = "What will " + State.variables.characters[this.turnOwner].name + " do?";

    this.randoCombatantExcept = function(exceptCharacter) {
        var combatants = this.enemyParty.concat(this.playerParty);
        var eligibleCombatants = this.combatants.filter(combatant => combatant.id != exceptCharacter.id);
        var unluckyIndex = Math.floor(Math.random() * eligibleCombatants.length);
        return eligibleCombatants[unluckyIndex];

    }
    /**
    Concat the enemy and player party arrays, and return result
    */
    this.getAllCombatants = function() {
        return this.enemyParty.concat(this.playerParty);
    }

    /**
    Handle upkeep related to a new round beginning (i.e. top of the round to ye!)
    */
    this.processRoundTop = function() {
        // tick down status effects
        for (let enemyChar of this.enemyParty) {
            var enemyCharacter = State.variables.characters[enemyChar.id];
            for (let effect of enemyCharacter.statusEffects) {
                if (enemyCharacter.living) {
                    // process top of stateffects with variable or triggered effects per round
                    if (effect.id === "terror") {
                        // terror's effect will roll percentage, with 35% to skip this turn
                        if (effect.effect()) {
                            enemyCharacter.combatFlags |= enemyCharacter.CombatFlags.FLAG_SKIP_TURN;
                        }
                    } else if (effect.id === "poison") {
                        enemyCharacter.stats["hp"] -= enemyCharacter.stats["maxHP"] * 0.1;
                    } else if (effect.id === "regen") {
                        State.variables.statusEffectsDict["regen"].effect(enemyCharacter);
                    }


                    effect.tickDown();
                    if (effect.ticks <= 0) {
                        // reverse the effect now that it is over
                        effect.reverseEffect(enemyCharacter);
                        // reset the ticks count to duration in case we
                        // want to re-use this statuseffect instance
                        effect.ticks = effect.duration;
                        // remove the status effect from the character
                        State.variables.removeStatusEffect(enemyCharacter, effect);
                    }
                } else {
                    // reverse the effect now that char is dead
                    effect.reverseEffect(enemyCharacter);
                    // reset the ticks count to duration in case we
                    // want to re-use this statuseffect instance
                    effect.ticks = effect.duration;
                    // remove the status effect from the character
                    State.variables.removeStatusEffect(enemyCharacter, effect);
                }
            }
        }
        for (let playerChar of this.playerParty) {
            var playerCharacter = State.variables.characters[playerChar.id];
            for (let effect of playerCharacter.statusEffects) {
                if (playerCharacter.living) {
                    // process top of stateffects with variable effects per round
                    if (effect.id === "terror") {
                        // terror's effect will roll percentage, with 35% to skip this turn
                        if (effect.effect()) {
                            playerCharacter.combatFlags |= playerCharacter.CombatFlags.FLAG_SKIP_TURN;
                        }
                    } else if (effect.id === "regen") {
                        State.variables.statusEffectsDict["regen"].effect(playerCharacter);
                    }

                    effect.tickDown();
                    if (effect.ticks <= 0) {
                        // reverse the effect now that it is over
                        effect.reverseEffect(playerCharacter);
                        // reset the ticks count to duration in case we
                        // want to re-use this statuseffect instance
                        effect.ticks = effect.duration;
                        // remove the status effect from the character
                        State.variables.removeStatusEffect(playerCharacter, effect);
                    }
                } else {
                    // reverse the effect now that character is dead
                    effect.reverseEffect(playerCharacter);
                    // reset the ticks count to duration in case we
                    // want to re-use this statuseffect instance
                    effect.ticks = effect.duration;
                    // remove the status effect from the character
                    State.variables.removeStatusEffect(playerCharacter, effect);
                }
            }
        }
    } // end processRoundTop fn

    /**
    Process the bottom of a combat round.  
    */
    this.processRoundBottom = function() {
        var dedEnemies = 0;
        var dedPlayers = 0;
        for (let enemyChar of this.enemyParty) {
            var enemyCharacter = State.variables.characters[enemyChar.id];
            for (let effect of enemyCharacter.statusEffects) {
                // process bottom of stateffects with variable effects per round
                if (effect.id === "terror") {
                    // clear the FLAG_SKIP_TURN flag
                    enemyCharacter.combatFlags &= ~enemyCharacter.CombatFlags.FLAG_SKIP_TURN;
                }
            }

            // check for death
            if (enemyCharacter.stats.hp <= 0) {
                dedEnemies++;
            }
        }
        for (let playerChar of this.playerParty) {
            // extract Character object from $characters
            var playerCharacter = State.variables.characters[playerChar.id];
            for (let effect of playerCharacter.statusEffects) {
                // process bottom of stateffects with variable effects per round
                if (effect.id === "terror") {
                    // clear the FLAG_SKIP_TURN flag
                    playerCharacter.combatFlags &= ~playerCharacter.CombatFlags.FLAG_SKIP_TURN;
                }
            }

            // check for death
            if (playerCharacter.stats.hp <= 0) {
                dedPlayers++;
            }
        }

        // check victory conditions -- if one is met, we do not need another turn so return false.  Else, combat continues so we return true.
        // todo: need a way to clear conditions and/or reset stats as desired at end of combat
        console.log("Dead players: " + dedPlayers + ", dead enemies: " + dedEnemies);
        if (dedEnemies >= this.enemyParty.length) {
            this.combatResult = this.CombatResultEnum.playerVictory;
            return false;
        } else if (dedPlayers >= this.playerParty.length) {
            this.combatResult = this.CombatResultEnum.enemyVictory;
            return false;
        } else {
            return true;
        }

    } // end processRoundBottom()
} // end Combat

State.variables.combatArena = undefined;

Objects in story javascript would be redefined during startup after save/load, right?

Given the above, my toJSON() would look something like the following?

State.variables.Combat.prototype.toJSON = function () {
	// Return a code string that will create a new instance
	// containing our current data.
	return JSON.reviveWrapper('new State.variables.Combat(State.variables.combatArena.playerParty, State.variables.combatArena.enemyParty, State.variables.combatArena.destPassageName)', this);
};

probably would be cleaner to add a copy constructor. 

by (159k points)

> what is ReviveData in this case and where would it be coming from?

If you read the SugarCube documentation for the JSON.reviveWrapper() function you will learn that $ReviveData$ is a special variable that allows the value given for the optional reviveData argument to be passed to the object/class instantiation String supplied as value of codeString argument.

eg. In the case of the third example supplied in the documentation

JSON.reviveWrapper('new Character($ReviveData$)', $pc._getData());

...the value of the reviveData argument, in this case the return of the $pc._getData() function, will be assigned to the special $ReviveData$ variable. The value of this special variable will then be passed as an argument to the object/class instantiation contains within the codeString.

by (420 points)
got it, thanks!  Can $ReviveData$ work with data of any type as long as it is serializable?  For instance, my Combat ctor expects arrays of complex Character objects; would I need to implement clone() and/or toJSON() methods for that class and its complex dependencies as well?

Also, does the JSON de/serialization process rely on both clone() and toJSON()?
by (68.6k points)

> Can $ReviveData$ work with data of any type as long as it is serializable?

Yes, with emphasis on "as long as it is serializable".

 

> […] would I need to implement clone() and/or toJSON() methods for that class and its complex dependencies as well?

If they're not either natively JSON serializable or been made so by SugarCube, then yes, you must implement the methods.

JSON natively supports: basic generic objects and Array.  SugarCube extends support to include: Date, Map, Set.

 

> Also, does the JSON de/serialization process rely on both clone() and toJSON()?

The JSON serialization process relies on .toJSON().  Depending on how you code that, it may rely itself on .clone().  Beyond that, the state history relies on .clone().  Thus, in most cases, you really do need both.

by (420 points)

I'm a bit confused as to what data toJSON and clone need for de/serialization to succeed -- is it only the ctor params?  Do the ctor params need to be sufficient to recreate the complete object state, or can JSON handle that on its own provided the object's attributes are themselves natively serializable by JSON?

I'm seeing two problems specifically:

1. Entity not defined, aborting error

I have the class Entity defind as follows:

function Entity(name){
	this.name = name;
	
	/**
	An array of strings representing domains of thought, emotion, imagination, and creativity associated with this Entity.  e.g. Foxfire entity associated with ["whimsy","illusion","seduction","manipulation","domination"]
	*/
	this.associatedDomains = [];
	
	/**
	A multi-dimensional array of Incarnations learned by wielders of this Entity at a level equivalent to the index of the Incarnation array.
	e.g. [[fire blast, ice blast],[],[rock tomb]] would indicate the wielder leanrs fire blast and ice blast at level 1, and rock tomb at level 3.
	*/
	this.associatedIncarnationsByLevel = [];
}// end Entity

for which I wrote the toJSON and clone methods:

/**
It is necessary to implement toJSON and clone for all complex objects to facilitate
JSON de/serialization
*/
Entity.prototype.toJSON = function () {
	// Return a code string that will create a new instance
	// of Entity with our current data.
	return JSON.reviveWrapper(String.format(
		'new Entity({0})',
		JSON.stringify(this.name)
	));
};
Entity.prototype.clone = function () {
	// Return a new instance containing our current data.
	return new Entity(
		this.name
	);
};

When I save/load with these methods defined, Twine gives me the error 'Entity is not defined. Aborting.'  Entity is only used as part of defining State.variables.Character objects:

State.variables.Character = function Character(id,name){
	...
	
	// a character's Entity is their inherent set of talents re: Incarnation.  Only the human can learn Incarnations from any Entity.
	this.entity = new Entity("unset");
	
	...
}// end Character
/**
It is necessary to implement toJSON and clone for all complex objects to facilitate
JSON de/serialization
*/
State.variables.Character.prototype.toJSON = function () {
	// Return a code string that will create a new instance
	// of Character with our current data.
	return JSON.reviveWrapper(String.format(
		'new State.variables.Character({0},{1})',
		JSON.stringify(this.id),
		JSON.stringify(this.name)
	));
};
State.variables.Character.prototype.clone = function () {
	// Return a new instance containing our current data.
	return new State.variables.Character(
		this.id,
		this.name
	);
};

 

2. Object definitions are known, but values are default

I implemented toJSON and clone for State.variables.Combat as follows:

State.variables.Combat = function Combat(playerParty,enemyParty,destPassageName){
	this.PlayerTurnState = Object.freeze (
		{ 
			selectTarget: 1, 
			selectAbility: 2,
			displayResults: 3,
			selectEnemyTarget: 4,
			selectAllyTarget: 5
		});
	this.EnemyTurnState = Object.freeze (
		{ 
			runAI: 1, 
			displayResults: 2
		});
	this.CombatResultEnum = Object.freeze (
		{ 
			pending: 0,
			playerVictory: 1, 
			enemyVictory: 2
		});
	
	// metadata about what state the player's turn is in.  This is necessary to help route the UI around the various actions a human player will need to input
	this.playerTurnState = this.PlayerTurnState.selectAbility;
	this.enemyTurnState = this.EnemyTurnState.runAI;
	this.playerParty = playerParty;
	this.enemyParty = enemyParty;
	this.destPassageName = destPassageName;
	this.combatResult = this.CombatResultEnum.pending;
	
	// tracks the currently selected player ability
	this.currentSelectedAbility = new State.variables.Ability("unset");
	
	// tracks the turn as either player or enemy group
	this.turnGroup = "player";
	
	// tracks the actual character id whose turn it is
	this.turnOwner = "player";
	
	// tracks the currently selected ability for the current turn owner
	this.abilityName = "";
	
	// tracks the target of the current ability
	this.currentTargetCharacter = new State.variables.Character("unset");
	
	// the text feedback to the user re: the state of combat
	this.combatLogContent = "What will "+State.variables.characters[this.turnOwner].name+" do?";
	
	this.randoCombatantExcept = function(exceptCharacter){
		var combatants = this.enemyParty.concat(this.playerParty);
		var eligibleCombatants = this.combatants.filter(combatant => combatant.id != exceptCharacter.id);
		var unluckyIndex = Math.floor(Math.random() * eligibleCombatants.length);
		return eligibleCombatants[unluckyIndex];
		
	}
	/**
	Concat the enemy and player party arrays, and return result
	*/
	this.getAllCombatants = function(){
		return this.enemyParty.concat(this.playerParty);
	}
	
	/**
	Handle upkeep related to a new round beginning (i.e. top of the round to ye!)
	*/
	this.processRoundTop = function(){
	// tick down status effects
	for(let enemyChar of this.enemyParty){
		var enemyCharacter = State.variables.characters[enemyChar.id];
			for(let effect of enemyCharacter.statusEffects){
				if(enemyCharacter.living){
					// process top of stateffects with variable or triggered effects per round
					if(effect.id === "terror"){
						// terror's effect will roll percentage, with 35% to skip this turn
						if(effect.effect()){
							enemyCharacter.combatFlags |= enemyCharacter.CombatFlags.FLAG_SKIP_TURN;	
						}
					}
					else if(effect.id === "poison"){
						enemyCharacter.stats["hp"] -= enemyCharacter.stats["maxHP"]*0.1; 	
					}
					else if(effect.id === "regen"){
						State.variables.statusEffectsDict["regen"].effect(enemyCharacter);	
					}
					
					
					effect.tickDown();
					if(effect.ticks <= 0){
						// reverse the effect now that it is over
						effect.reverseEffect(enemyCharacter);	
						// reset the ticks count to duration in case we
						// want to re-use this statuseffect instance
						effect.ticks = effect.duration;
						// remove the status effect from the character
						State.variables.removeStatusEffect(enemyCharacter,effect);
					}
				}else{
					// reverse the effect now that char is dead
					effect.reverseEffect(enemyCharacter);	
					// reset the ticks count to duration in case we
					// want to re-use this statuseffect instance
					effect.ticks = effect.duration;
					// remove the status effect from the character
					State.variables.removeStatusEffect(enemyCharacter,effect);
				}
			}
	}
	for(let playerChar of this.playerParty){
		var playerCharacter = State.variables.characters[playerChar.id];
			for(let effect of playerCharacter.statusEffects){
				if(playerCharacter.living){
					// process top of stateffects with variable effects per round
					if(effect.id === "terror"){
						// terror's effect will roll percentage, with 35% to skip this turn
						if(effect.effect()){
							playerCharacter.combatFlags |= playerCharacter.CombatFlags.FLAG_SKIP_TURN;	
						}
					}
					else if(effect.id === "regen"){
						State.variables.statusEffectsDict["regen"].effect(playerCharacter);	
					}
					
					effect.tickDown();
					if(effect.ticks <= 0){
						// reverse the effect now that it is over
						effect.reverseEffect(playerCharacter);	
						// reset the ticks count to duration in case we
						// want to re-use this statuseffect instance
						effect.ticks = effect.duration;
						// remove the status effect from the character
						State.variables.removeStatusEffect(playerCharacter,effect);
					}
				}else{
					// reverse the effect now that character is dead
					effect.reverseEffect(playerCharacter);	
					// reset the ticks count to duration in case we
					// want to re-use this statuseffect instance
					effect.ticks = effect.duration;
					// remove the status effect from the character
					State.variables.removeStatusEffect(playerCharacter,effect);
				}
			}
	}
 }// end processRoundTop fn
	
	/**
	Process the bottom of a combat round.  
	*/
	this.processRoundBottom = function(){
		var dedEnemies = 0;
		var dedPlayers = 0;
		for(let enemyChar of this.enemyParty){
			var enemyCharacter = State.variables.characters[enemyChar.id];
			for(let effect of enemyCharacter.statusEffects){
				// process bottom of stateffects with variable effects per round
				if(effect.id === "terror"){
					// clear the FLAG_SKIP_TURN flag
		enemyCharacter.combatFlags &= ~enemyCharacter.CombatFlags.FLAG_SKIP_TURN;
				}
			}
			
			// check for death
			if(enemyCharacter.stats.hp <= 0){
				dedEnemies++;	
			}
		}
		for(let playerChar of this.playerParty){
			// extract Character object from $characters
			var playerCharacter = State.variables.characters[playerChar.id];
			for(let effect of playerCharacter.statusEffects){
				// process bottom of stateffects with variable effects per round
				if(effect.id === "terror"){
					// clear the FLAG_SKIP_TURN flag
		playerCharacter.combatFlags &= ~playerCharacter.CombatFlags.FLAG_SKIP_TURN;
				}
			}
			
			// check for death
			if(playerCharacter.stats.hp <= 0){
				dedPlayers++;	
			}
		}
		
		// check victory conditions -- if one is met, we do not need another turn so return false.  Else, combat continues so we return true.
		// todo: need a way to clear conditions and/or reset stats as desired at end of combat
		console.log("Dead players: "+dedPlayers+", dead enemies: "+dedEnemies);
		if(dedEnemies >= this.enemyParty.length){
			this.combatResult = this.CombatResultEnum.playerVictory;
			return false;	
		}else if(dedPlayers >= this.playerParty.length){
			this.combatResult = this.CombatResultEnum.enemyVictory;
			return false;	
		}else{
			return true;	
		}
		
	}// end processRoundBottom()
}// end Combat

/**
It is necessary to implement toJSON and clone for all complex objects to facilitate
JSON de/serialization
*/
State.variables.Combat.prototype.toJSON = function () {
	// Return a code string that will create a new instance
	// of Combat with our current data.
	return JSON.reviveWrapper(String.format(
		'new State.variables.Combat({0},{1},{2})',
		JSON.stringify(this.playerParty),
		JSON.stringify(this.enemyParty),
		JSON.stringify(this.destPassageName)
	));
};
State.variables.Combat.prototype.clone = function () {
	// Return a new instance containing our current data.
	return new State.variables.Combat(
		this.playerParty,
		this.enemyParty,
		this.destPassageName
	);
};

This allows me to get into a combat scenario after save/load, but the parameters (such as Character objects' HP and Incarnation costs were default values.  I'm fairly sure the issue here is that I need to be able to restore the complete object state from the ctor that JSON deserialization will call with the data given to JSON serialization -- is that correct?

Thanks so much for all the help! If you're curious or need to see all the code, the Twine HTML is at https://github.com/crescojeff/Fuzziolump and published HTML is hosted at https://crescojeff.github.io/Fuzziolump/

by (68.6k points)

> I'm a bit confused as to what data toJSON and clone need for de/serialization to succeed -- is it only the ctor params?

Both methods must account for all properties whose values should be copied into the new instance.  Your current .clone() and .toJSON() methods are not populating the new instance of your object with all of the original's data (I'm assuming the domains and incarnations are necessary), so that's an issue.

 

> Entity not defined…

You have not defined Entity in such a way so that it is within scope of the revival process, so it's literally not defined when the system attempts to call it during the revival process.  Rather than using a function declaration to define the constructor, try a function expression which you also assign to an auto-global.

 

Here's one example of how you could rewrite them to resolve both issues:

window.Entity = function (name, domains, incarnations) {
	this.name = name;
	this.associatedDomains = domains || [];
	this.associatedIncarnationsByLevel = incarnations || [];
};

Entity.prototype.clone = function () {
	return new Entity(
		this.name,
		clone(this.associatedDomains),
		clone(this.associatedIncarnationsByLevel)
	);
};

Entity.prototype.toJSON = function () {
	return JSON.reviveWrapper(String.format(
		'new Entity({0},{1},{2})',
		JSON.stringify(this.name),
		JSON.stringify(this.associatedDomains),
		JSON.stringify(this.associatedIncarnationsByLevel)
	));
};

Alternatively.  You could make your constructor take a config/option object parameter (a simple property bag), which could simplify its associated .clone() and .toJSON() methods.  Not a huge deal here, but on classes with more properties to account for it could be nice (though that might mean you need alter how you invoke them elsewhere).  An answer I gave to CeLezte gives an example of that.

 

A further issue.  You really should not assign your Character class definition to $Character (or, for that matter, any class definition to any story variable).  I suggest using a setup similar to Entity for the rest of your class definitions and only assign instances of them to story variables, as necessary.

 

> […], but the parameters (such as Character objects' HP and Incarnation costs were default values.  I'm fairly sure the issue here is that I need to be able to restore the complete object state from the ctor that JSON deserialization will call with the data given to JSON serialization -- is that correct?

Yes.

 

by (420 points)
edited by

Sounds good, thanks!

In the proposed Entity clone() method

Entity.prototype.clone = function () {
	return new Entity(
		this.name,
		clone(this.associatedDomains),
		clone(this.associatedIncarnationsByLevel)
	);
};

why do you clone() the two arrays?  Is the clone() function in clone(this.associatedDomains) built into Sugarcube?  

On a probably related note, if I were to pass the entire current instance of an object as the argument for $ReviveData$ in toJSON() , would I need to make a deep copy in the copy ctor or would a shallow copy suffice?

E.g.:

window.Combat = function Combat(playerParty, enemyParty, destPassageName, copy) {
    // normal parameterized construction
    if (copy == undefined) {

        this.PlayerTurnState = Object.freeze({
            selectTarget: 1,
            selectAbility: 2,
            displayResults: 3,
            selectEnemyTarget: 4,
            selectAllyTarget: 5
        });
        ...

        this.playerTurnState = this.PlayerTurnState.selectAbility;
        this.enemyTurnState = this.EnemyTurnState.runAI;
        this.playerParty = playerParty;
        this.enemyParty = enemyParty;
        this.destPassageName = destPassageName;
        this.combatResult = this.CombatResultEnum.pending;

        ...

        this.randoCombatantExcept = function(exceptCharacter) {
            var combatants = this.enemyParty.concat(this.playerParty);
            var eligibleCombatants = this.combatants.filter(combatant => combatant.id != exceptCharacter.id);
            var unluckyIndex = Math.floor(Math.random() * eligibleCombatants.length);
            return eligibleCombatants[unluckyIndex];

        }
        ...
    } // end if copy is not defined
    else {
        // make a shallow copy from given instance
        Object.assign(this, copy);
    }
} // end Combat
window.Combat.prototype.toJSON = function() {
    return JSON.reviveWrapper('new window.Combat(undefined,undefined,undefined,$ReviveData$)',this);
};
window.Combat.prototype.clone = function() {
    // Return a new instance containing our current data.
    return new window.Combat(
        undefined,
        undefined,
        undefined,
        this
    );
};

That ctor adaptation based on args looks pretty wretched, but I'm new to JS and I think that might be the only way to support both explicit params and a copy ctor in the absence of function overloading; please advise if there's a more elegant way.

>A further issue.  You really should not assign your Character class definition to $Character (or, for that matter, any class definition to any story variable).  I suggest using a setup similar to Entity for the rest of your class definitions and only assign instances of them to story variables, as necessary.

Will do, but what's the reason for this?

by (68.6k points)
edited by

EDIT (2018-12-15): Fixed the example (again).

 

> […] why do you clone() the two arrays?  Is the clone() function in clone(this.associatedDomains) built into Sugarcube?

To make deep copies of the arrays and their contents.  And yes, the clone() function is a SugarCube built-in.

 

> On a probably related note, if I were to pass the entire current instance of an object as the argument for $ReviveData$ in toJSON() , would I need to make a deep copy in the copy ctor or would a shallow copy suffice?

It depends on what you're copying.  There's no one size fits all answer, though when in doubt a deep copy is probably the safe option.

 

To pause for a moment on your example.  You're doing very odd things in your constructor.  For example:

The this.PlayerTurnState own property is a frozen generic object which only contains primitive values.  There doesn't seem any reason for that to be an own property of the instances; or even a prototype property, though it would make more sense attached there.  It would be better as a static property of the class.

By their names alone, the this.EnemyTurnState and this.CombatResultEnum properties seem like more candidates for the static property treatment.

The this.randoCombatantExcept own method is similar.  It's not a closure over some state internal to its parent function (the constructor), so there doesn't seem any reason for it to be an own property.  It would be better placed on the prototype.

Additionally, within this.randoCombatantExcept.  You declare combatants but use this.combatants on the next line, I'm betting that latter should be combatants instead.  I'd also suggest not mixing ES6 arrow functions into what seems to be an, essentially, ES5 codebase.

 

> That ctor adaptation based on args looks pretty wretched, but I'm new to JS and I think that might be the only way to support both explicit params and a copy ctor in the absence of function overloading; please advise if there's a more elegant way.

There are several ways you could go about it.  The easiest is probably to use a config/option object parameter, rather than explicit parameters.  Though, this requires you alter how you normally instantiate the class.

/*
	Combat class constructor.
*/
window.Combat = function Combat(config) {
	/* ... */

	// Copy an instance of Combat.
	//
	// We know it is/was an instance of Combat because it
	// has a property that only an instance would have.
	// In other words, the generic object you pass in to
	// create a new instance will never have that property.
	// E.g. `playerTurnState` seems like a good candidate.
	if (config.hasOwnProperty('playerTurnState')) {
		// NOTE: Only use one of the following examples!

		// Either copy the own properties individually, for maximum control.
		this.playerParty     = clone(config.playerParty);
		this.enemyParty      = clone(config.enemyParty);
		this.destPassageName = clone(config.destPassageName);
		this.playerTurnState = clone(config.playerTurnState);
		this.enemyTurnState  = clone(config.enemyTurnState);
		this.combatResult    = clone(config.combatResult);

		// Or copy the own properties automatically, for simplicity.
		Object.keys(config).forEach(function (pn) {
			this[pn] = clone(config[pn]);
		}, this);
	}
	// Create a new instance of Combat.
	else {
		// Initialize from the config object parameter.
		this.playerParty     = config.playerParty;
		this.enemyParty      = config.enemyParty;
		this.destPassageName = config.destPassageName;

		// Initialize from defaults and whatnot.
		this.playerTurnState = Combat.PlayerTurnState.selectAbility;
		this.enemyTurnState  = Combat.EnemyTurnState.runAI;
		this.combatResult    = Combat.CombatResultEnum.pending;
	}

	/* ... */
};

You could instantiate the class with an object literal.  For example:

new Combat({
	playerParty : /* ... */,
	enemyParty  : /* ... */,
	destPassage : /* ... */
});

 

EDIT: Note, the above example may not be complete.  When making a copy of an existing instance within the constructor, you're going to need to clone/deep-copy any own properties which are not primitives; though cloning everything might be easier.

For bonus points.  Based on my notes above, the rest of the class might look like:

/*
	Combat class prototype methods and properties.
*/
window.Combat.prototype.clone = function () {
	return new window.Combat(this);
};

window.Combat.prototype.toJSON = function () {
	// NOTE: Do not use `this` directly or you'll trigger
	// out of control recursion.  Make a copy of the own
	// properties instead.
	var data = {};
	Object.keys(this).forEach(function (pn) {
		data[pn] = clone(this[pn]);
	}, this);
	return JSON.reviveWrapper('new window.Combat($ReviveData$)', data);
};

window.Combat.prototype.randoCombatantExcept = function (exceptCharacter) {
	var eligibleCombatants = this.enemyParty.concat(this.playerParty)
		.filter(function (combatant) { return combatant.id !== exceptCharacter.id; });
	return eligibleCombatants[Math.floor(Math.random() * eligibleCombatants.length)];
};

/*
	Combat class static methods and properties.
*/
window.Combat.PlayerTurnState = Object.freeze({
	selectTarget      : 1,
	selectAbility     : 2,
	displayResults    : 3,
	selectEnemyTarget : 4,
	selectAllyTarget  : 5
});

window.Combat.EnemyTurnState = Object.freeze({
	/* ... */
});

window.Combat.CombatResultEnum = Object.freeze({
	/* ... */
});

 

> Will do, but what's the reason for this?

You neither need nor want to stuff static data and/or behaviors into story variables because each moment within the history gets a copy of them, and the history itself is serialized within the current play session and saves.  Doing so bloats the history, and thus the current session and saves, for no good reason or purpose.

 

EDIT (2018-12-15): Fixed the example (again).

 

by (68.6k points)
I screwed up the examples in my last reply, so this is just a note to say they've been fixed.
...