Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Object created from a "class" object from ES6 has it's functions broken after page F5



Using SugarCube 2.7.1, compiled with TweeGo 0.99.0, tested on Chrome 51.0 (64bit)

It seems object defined with ES6 class keywords gets murdered when pressing F5. It's content seems to be find, but the prototype is completely overriden by what looks to be Object's.

Sample implementation
:: Db [script]

window.ns = {};//namespace
window.ns.Db = class NpcDb { 
    constructor() {
        this.value = 100;
    }
    giveValue() {
        return this.value;
    }
}

In init:
:: StoryInit
<<set $db to new ns.Db()>>

In passage:
:: Sample [nobr]
Test.
<<print $db.giveValue()>>

First load:
Test. 100
Via Chrome Developper Console
Object.getOwnPropertyNames(SugarCube.State.active.variables.db.__proto__);
["constructor", "giveValue"]

Then I press F5.
Test. Error: <<print>>: bad evaluation: State.variables.db.giveValue is not a function
Object.getOwnPropertyNames(SugarCube.State.active.variables.db.__proto__);
["hasOwnProperty", "constructor", "toString", "toLocaleString", "valueOf", "isPrototypeOf", "propertyIsEnumerable", "__defineGetter__", "__lookupGetter__", "__defineSetter__", "__lookupSetter__", "__proto__"]

What could I do to not have $db butchered? Is there a problem with SugarCube 2.7.1, TweeGo 0.99.0 and these ES6 keywords?

Comments

  • You need to define clone() and toJSON() methods for your class. For example:
    window.ns.Db = class NpcDb { 
    	constructor(dbObj) {
    		this._db = Object.assign({
    			value : 100
    		}, dbObj);
    	}
    
    	/* Allows the object to be properly cloned from passage to passage. */
    	clone() {
    		return new ns.Db(clone(this._db));
    	}
    
    	/* Allows the object to be properly restored from serializations. */
    	toJSON() {
    		return JSON.reviveWrapper('new ns.Db(' + JSON.stringify(this._db) + ')');
    	}
    
    	giveValue() {
    		return this._db.value;
    	}
    };
    
    You don't absolutely need to move your data into an internal object, however, it does help make the two methods easier to write.

    I don't have enough energy at the moment to explain the why of it.
  • edited August 2016
    So from what I understand, when changing passages, .clone is called on each object in the State.variables(). I imagine it's a shallow clone that doesn't bother recreating the underlying Object. It just does foreach(property in that) { this[property] = that[property] }.

    And I see that toJSON returns a new ns.Db, so the functions should always reflect what is in the code, correct? I.e I edit my JS to add/change/remove a function.

    I'll fiddle with this a bit, it would be very helpful to be able to call methods from the object directly, instead of using a "DbManipulator" class like I implemented
    ns.DbManipulator.giveValue = function(db) { return db.value; }
    
  • sssk wrote: »
    I imagine it's a shallow clone that doesn't bother recreating the underlying Object. It just does foreach(property in that) { this[property] = that[property] }.
    Traversing between passages actually results in a deep copy/clone, which can be demonstrated using the following two passages:

    1. First passage:
    <<set $npc to {"name": "John"}>>\
    <<set $party to {"leader": $npc}>>\
    \
    NPC Name: <<= $npc.name>>
    Party Leader: <<= $party.leader.name>>
    
    <<set $npc.name to "Jane">>\
    NPC Name: <<= $npc.name>>
    Party Leader: <<= $party.leader.name>>
    
    <<set $party.leader.name to "Bob">>\
    NPC Name: <<= $npc.name>>
    Party Leader: <<= $party.leader.name>>
    
    [[Next Passage|Second]]
    
    2. Second passage:
    NPC Name: <<= $npc.name>>
    Party Leader: <<= $party.leader.name>>
    
    <<set $npc.name to "Jane">>\
    NPC Name: <<= $npc.name>>
    Party Leader: <<= $party.leader.name>>
    
    <<set $party.leader.name to "John">>\
    NPC Name: <<= $npc.name>>
    Party Leader: <<= $party.leader.name>>
    
  • Yea deep copy would make sense to be by default, idk why I thought it wasn't :pensive:

    By the way, do you know the mechanics behind:
    You don't absolutely need to move your data into an internal object, however, it does help make the two methods easier to write.

    I don't have enough energy at the moment to explain the why of it.
  • I only have intermediate knowledge about Javascript but I believe one of the reasons is that the encapsulated data object only contains 'value' properties (no directly 'method' properties) which makes it a light weight object and thus more efficient to clone.
  • edited August 2016
    Aren't Objects passed as references if sent through functions? Unless you mean the clone itself is going to have a hard time on an Object + prototype vs a Object with only properties?

    Also I can't find any info on Google about creating a copy-constructor in Javascript (aka constructor(copy) { this = copy.clone(); }), whats the rundown to avoid using an internal object?
  • Actually the problem I have converting it is
    clone() { return new ns.Db(clone(this._db)); }
    
    causes a stack overflow when I pass it "this". clone() isn't a standard JS method so I don't know what it does, and googling doesn't give much for "sugarcube clone".
  • That clone() method is part of the SugarCube 2 story format engine.

    Normally I would suggest looking at SugarCube 2's documention and in paticular the Story Functions or the Native Object Methods sections for information on the story format's Javascript related methods but the clone() function is undocumented so to findout information about that method you will need to look at the relevant source code.
  • edited August 2016
    Well thats obvious with the source, clone(obj) does obj.clone() if it exists. And I defined obj.clone() as clone(obj). So stackoverflow. Thanks for the pointer.

    ...looks like the samething happens to toJSON, tries to JSON itself or something.

    I'll just implement it with an internal container for now.

    Though I'dd love to know whats going on with "(revive:eval),(class Db { ..." causing an infinite loop of toJSON. Even if you where to eval the class declaration it shouldn't call the contained functions.
  • Ok this is hell, I have to expose each value inside internal object, so many getter and setters...
  • edited August 2016
    sssk wrote: »
    So from what I understand, when changing passages, .clone is called on each object in the State.variables(). I imagine it's a shallow clone that doesn't bother recreating the underlying Object. It just does foreach(property in that) { this[property] = that[property] }.
    No. It's a deep copy, actually.

    sssk wrote: »
    And I see that toJSON returns a new ns.Db, so the functions should always reflect what is in the code, correct? I.e I edit my JS to add/change/remove a function.
    I have no idea what you're asking here.

    The code string in my example toJSON() method uses new ns.Db() since that's how you create a new instance of the class—the code string will be evaluated later to revive the instance it was called upon, which will happen in a different scope, so you have to use the globally accessible identifier, ns.Db.

    You may add whatever methods to your class you chose. Neither the clone() or toJSON() methods need to know about the rest of your class' behavior, since they create new instances of the class with copies of the existing instance's data—well, toJSON() doesn't, but that's what the code string it returns enables.

    I'm unsure if I answered your question.

    sssk wrote: »
    I'll fiddle with this a bit, it would be very helpful to be able to call methods from the object directly, instead of using a "DbManipulator" class like I implemented
    ns.DbManipulator.giveValue = function(db) { return db.value; }
    
    You can. Why do you think you can't?

    sssk wrote: »
    By the way, do you know the mechanics behind:
    You don't absolutely need to move your data into an internal object, however, it does help make the two methods easier to write.

    I don't have enough energy at the moment to explain the why of it.
    The clone() and toJSON() methods need to create a new copy of the instance they're called upon—indirectly in the case of toJSON(), as explained previously. To do so they need to be able to call the class' constructor to instantiate a new object and then to copy the original instance's data over to the new instance, thus finally yielding a full copy of the original instance.

    You can pass the class' own data properties in a number of ways. My original example showed how to use a simple object, just a data property bag, to hold the class' data properties so that writing the clone() and toJSON() methods was very simple.

    As I noted, however, you do not have to do it that way. For example, using loose properties:
    window.ns.Db = class NpcDb { 
    	constructor() {
    		// Our own data properties.
    		this.value = 100;
    		this.foo   = 'bar';
    		this.elf   = false;
    	}
    
    	getValue() {
    		return this.value;
    	}
    
    	getFoo() {
    		return this.foo;
    	}
    
    	isElf() {
    		return this.elf;
    	}
    
    	/******************************************************************/
    
    	/*
    		Returns a simple object encapsulating our own data properties.
    	*/
    	_getData() {
    		// This must include all of our own data properties.
    		var data = {
    			value : this.value,
    			foo   : this.foo,
    			elf   : this.elf
    		};
    
    		return data;
    	}
    
    	/*
    		Copies values from the given object to our own data properties.
    		Only our own property names are copied.
    
    		Returns a self-reference to allow `toJSON()` to be simpler.
    	*/
    	_setData(data) {
    		Object.keys(this).forEach(function (key) {
    			this[key] = clone(data[key]);
    		}, this);
    
    		return this;
    	}
    
    	/*
    		Allows the object to be properly cloned from passage to passage.
    	*/
    	clone() {
    		var copy = new ns.Db();
    
    		copy._setData(this._getData());
    
    		return copy;
    	}
    
    	/*
    		Allows the object to be properly restored from serializations.
    	*/
    	toJSON() {
    		return JSON.reviveWrapper('(new ns.Db())._setData('
    			+ JSON.stringify(this._getData()) + ')');
    	}
    };
    
    Pro: Since it uses doesn't hold its data in a simple object internally, you may access the data properties directly.
    Con: As you can see, it make things a bit more complex. Either clone() and toJSON() need to be more complex or, as I did here, there needs to be additional helper methods to allow them to remain simple.

    To contrast, I've updated my original example to be similar in the data it holds:
    window.ns.Db = class NpcDb {
    	constructor(dbObj) {
    		this._db = Object.assign({
    			// Our own data properties.
    			value : 100,
    			foo   : 'bar',
    			elf   : false
    		}, dbObj);
    	}
    
    	getValue() {
    		return this._db.value;
    	}
    
    	getFoo() {
    		return this._db.foo;
    	}
    
    	isElf() {
    		return this._db.elf;
    	}
    
    	/******************************************************************/
    
    	/* Allows the object to be properly cloned from passage to passage. */
    	clone() {
    		return new ns.Db(clone(this._db));
    	}
    
    	/* Allows the object to be properly restored from serializations. */
    	toJSON() {
    		return JSON.reviveWrapper('new ns.Db(' + JSON.stringify(this._db) + ')');
    	}
    };
    
    Pro: The definitions of clone() and toJSON() are much more simple.
    Con: To access the data properties outside of method calls and/or getters/setters, you have to reference (instance)._db.(property), e.g. $db._db.foo.

    Both styles work. It's just a matter of where you want the complexity to be.

    sssk wrote: »
    Actually the problem I have converting it is
    clone() { return new ns.Db(clone(this._db)); }
    
    causes a stack overflow when I pass it "this". clone() isn't a standard JS method so I don't know what it does, and googling doesn't give much for "sugarcube clone".
    You don't pass it this. Never pass to the clone() function a reference to an object which has a clone() method if that method itself calls the clone() function, you'll start an infinite recursion chain and overflow the call stack.

    That is why my original example passed a simple data-only object to the clone() function, rather than a reference to the class instance.

    For example, the wrong way:
    clone(instance) → instance.clone() → clone(instance) → … → STACK_OVERFLOW
    
    And the right way:
    clone(instance) → instance.clone() → clone(data) → return
    

    My alternative example within this post shows another way to do it so that you don't have to use an internal data object for the class itself, but you'll still have to use a simple data object for the clone() and toJSON() methods.

    sssk wrote: »
    Though I'dd love to know whats going on with "(revive:eval),(class Db { ..." causing an infinite loop of toJSON. Even if you where to eval the class declaration it shouldn't call the contained functions.
    It likely isn't. Without looking at the code—it's been a while—what's likely happening is that since the incoming object is cloned as a matter of course, your broken clone() method is causing a stack overflow at that point.
  • Okay I have it working, I see what step I was missing, the get/setData. Thanks for the complete answer.

    Concerning the _getData(), instead of manually listing each properly, isn't it possible to do:
    Object.keys(this).forEach(key => copy[key] = clone(this[key]);
    

    To the same effet? I could understand if I wanted only certain properties, but I can't see the downside of using forEach if I want all the properties.
  • sssk wrote: »
    Concerning the _getData(), instead of manually listing each properly, isn't it possible to do:
    Object.keys(this).forEach(key => copy[key] = clone(this[key]);
    
    To the same effet? I could understand if I wanted only certain properties, but I can't see the downside of using forEach if I want all the properties.
    You have a missing parenthesis there.

    Yes, you could—which would definitely ease the maintenance burden. Though, I don't know if I'd call clone() there, since _setData() calls it.

    I would also be clear by calling the data object something like data, rather than copy, to avoid potential confusion.

    For example (which I'll do completely as ES6):
    	/*
    		Returns a simple object encapsulating our own data properties.
    	*/
    	_getData() {
    		const data = {};
    
    		Object.keys(this).forEach(key => data[key] = this[key]);
    
    		return data;
    	}
    
    	/*
    		Copies values from the given object to our own data properties.
    		Only our own property names are copied.
    
    		Returns a self-reference to allow `toJSON()` to be simpler.
    	*/
    	_setData(data) {
    		Object.keys(this).forEach(key => this[key] = clone(data[key]));
    
    		return this;
    	}
    

    PS – Caution: To other potential authors reading this and thinking "I'm going to do that too". As of this writing, I do not yet recommend using ES2015/ES6 in your stories due to legacy browser issues.
Sign In or Register to comment.