Commit 41f81d18 authored by Leon Arnott's avatar Leon Arnott
Browse files

Fixed a bug where the new SessionStorage system would interfere with the "test...

Fixed a bug where the new SessionStorage system would interfere with the "test story starting here" feature in Twine debugging.

Now, that feature is ignored if the debug option is enabled.

Also, if the (loadgame:) macro encounters an error while loading save data (such as, a passage it refers to no longer exists in this version of the story) then a user-facing prompt() will appear suggesting that the save data might be outdated, and asking the reader whether or not the save data should be deleted. It's expected that this message should only appear during story testing, or when upgrading to later versions of a story.
parent 7ff152207d6c
......@@ -18,7 +18,8 @@ Rough documentation is at http://twine2.neocities.org/. See below for compilatio
####Alterations
* Now, when playing, the current game session will attempt to preserve itself across browser reloads and back-forward navigation using browser SessionStorage. This means that reloading the page (without closing the window or tab) should also automatically reload the player's position in the story, as if by `(load-game:)`. This does not apply when using `(reload:)`, however, which always returns the story to the beginning.
* Now, when playing, the current game session will attempt to preserve itself across browser reloads and back-forward navigation using browser SessionStorage. This means that reloading the page (without closing the window or tab) should also automatically reload the player's position in the story, as if by `(load-game:)`. This does not apply when using `(reload:)`, however, which always returns the story to the beginning. If an error occurs while loading this data (such as, a passage it refers to no longer exists in this version of the story) then it will be silently ignored.
* If the `(loadgame:)` macro encounters an error while loading save data (such as, a passage it refers to no longer exists in this version of the story) then a polite dialog box (a simple JavaScript `prompt()`) will appear suggesting that the save data might be outdated, and asking the reader whether or not the save data should be deleted.
* The `(replace:)`, `(append:)` and `(prepend:)` macros now no longer target any hooks or text that haven't been rendered yet - so, `(replace: "cool")[hot] cool water` won't work because the `(replace:)` runs before "cool water" has rendered, but `cool water (replace: "cool")[hot]` and something like `(link: "heat")[(replace: "cool")[hot]] cool water` will. This finally normalises what was formerly very inconsistent behaviour across these three macros - `(replace:)` couldn't target forthcoming hooks but could target later text, and `(append:)` and `(prepend:)` would do the others' behaviour on forthcoming hooks.
* The `(text:)` macro now has another alias, `(str:)`. This alias will now be the preferred name for this macro in the documentation, mainly due to the arrival of other string macros that begin with "str-", and additionally to avoid semantic conflict with the various "text-" changer macros like `(text-style:)`.
* To more clearly separate the concepts of "printing data" and "running commands" in Harlowe, the `(print:)` macro will no longer run commands passed to it (that is, `(print:(go-to:"Foo"))` and `(go-to:"Foo")` will no longer do the same thing - the former will just print out a descriptive string, as if printing out a changer). Commands can now only be run by placing them directly in the passage (either as plain calls, inside variables, or wrapped in strings that (print:) receives).
......
......@@ -132,7 +132,7 @@ define(['jquery', 'utils', 'utils/selectors', 'state', 'section', 'passages'],
Early exit: the wrong passage name was supplied.
Author error must never propagate to this method - it should have been caught earlier.
*/
if (!passageData || !(passageData instanceof Map) || !passageData.has('source')) {
if (!Passages.hasValid(name)) {
impossible("Engine.showPassage", "There's no passage with the name \""+name+"\"!");
}
......
......@@ -145,17 +145,15 @@ require(['jquery', 'debugmode', 'renderer', 'state', 'engine', 'passages', 'util
$(document.head).append('<style data-title="Story stylesheet ' + (i + 1) + '">' + $(this).html());
});
// Load the sessionStorage if it's present
const sessionData = State.hasSessionStorage && sessionStorage.getItem("Saved Session");
// Load the sessionStorage if it's present (and we're not testing)
const sessionData = !Engine.options.debug && State.hasSessionStorage && sessionStorage.getItem("Saved Session");
if (sessionData) {
try {
State.deserialise(sessionData);
// If deserialisation fails (i.e. it returned an Error instead of true),
// it means the sessionData is invalid. Just ignore it - it's only temporary data.
if (State.deserialise(sessionData) === true) {
// This is copied from (load-game:).
Engine.showPassage(State.passage, false /* stretchtext value */);
return;
} catch(e) {
// Currently, failing to deserialise session storage is a silent error,
// owing to how temporary sessions are.
}
}
......
......@@ -67,7 +67,7 @@ define(['jquery', 'requestAnimationFrame', 'macros', 'utils', 'utils/selectors',
This and the next check must be made now, because the Passages
datamap could've been tinkered with since this was created.
*/
if (!Passages.has(name)) {
if (!Passages.hasValid(name)) {
return TwineError.create("macrocall",
"I can't (display:) the passage '"
+ name
......@@ -169,7 +169,7 @@ define(['jquery', 'requestAnimationFrame', 'macros', 'utils', 'utils/selectors',
/*
First, of course, check for the passage's existence.
*/
if (!Passages.has(name)) {
if (!Passages.hasValid(name)) {
return TwineError.create("macrocall",
"I can't (go-to:) the passage '"
+ name
......@@ -618,10 +618,22 @@ define(['jquery', 'requestAnimationFrame', 'macros', 'utils', 'utils/selectors',
save, and going to the passage where that save was made.
This macro assumes that the save slot exists and contains a game, which you can check by seeing if
`(saved-games:) contains` the slot name before running (load-game:).
`(saved-games: ) contains` the slot name before running (load-game:).
This command can't have changers attached - attempting to do so will produce an error.
In the event that the saved data exists, but contains an error - for instance, if it refers to a passage
which doesn't exist in this story, which could happen if one version of the story is used to save it, and
another is used to open it - then a polite dialog box will appear asking the reader whether or not the data
should be deleted. An example of such a dialog is below.
<blockquote>
Sorry to interrupt... The story tried to load saved data, but there was a problem.
The data refers to a passage named 'example', but it isn't in this story.<br><br>
That data might have been saved from a different version of this story. Should I delete it?<br>
(Type 'delete' and choose OK to delete it.)<br><br>
Either way, the story will now continue without loading the data.
</blockquote>
See also:
(save-game:), (saved-games:)
......@@ -636,12 +648,25 @@ define(['jquery', 'requestAnimationFrame', 'macros', 'utils', 'utils/selectors',
return TwineError.create("saving", "I can't find a save slot named '" + slotName + "'!");
}
State.deserialise(saveData);
/*
There's not a strong reason to check for the destination passage existing,
because (save-game:) can only be run inside a passage. If this fails,
the save itself is drastically incorrect.
If this returns false, the save itself is drastically incorrect.
*/
const result = State.deserialise(saveData);
if (result instanceof Error) {
/*
Since this could be an issue with multiple versions of the same story,
showing a TwineError, a developer-facing error, seems incorrect. So, instead
a simple confirm() is shown, offering to delete the data.
*/
if ("delete" === (window.prompt("Sorry to interrupt... The story tried to load saved data, but there was a problem.\n"
+ result.message
+ "\n\nThat data might have been saved from a different version of this story. Should I delete it?"
+ "\n(Type 'delete' and choose OK to delete it.)"
+ "\n\nEither way, the story will now continue without loading the data.", '') || '').trim().toLowerCase()) {
localStorage.removeItem(storagePrefix("Saved Game") + slotName);
}
return;
}
requestAnimationFrame(Engine.showPassage.bind(Engine, State.passage, false /* stretchtext value */));
},
[String], false)
......
......@@ -800,7 +800,7 @@ define(['jquery', 'utils', 'utils/selectors', 'utils/operationutils', 'engine',
/*
First, of course, check for the passage's existence.
*/
if (!Passages.has(passage)) {
if (!Passages.hasValid(passage)) {
return TwineError.create("macrocall",
"I can't (" + name + ":) the passage '" + passage + "' because it doesn't exist."
);
......
......@@ -303,7 +303,7 @@ define(['jquery', 'macros', 'utils', 'utils/selectors', 'state', 'passages', 'en
/*
Check that the passage is indeed available.
*/
if (!Passages.has(passageName)) {
if (!Passages.hasValid(passageName)) {
/*
Since the passage isn't available, create a broken link.
TODO: Maybe this should be an error as well??
......@@ -531,7 +531,7 @@ define(['jquery', 'macros', 'utils', 'utils/selectors', 'state', 'passages', 'en
As explained above, we create the broken link now, and dispose of
whatever the contained hook had.
*/
if (!Passages.has(passageName)) {
if (!Passages.hasValid(passageName)) {
desc.source = '<tw-broken-link passage-name="' + Utils.escape(passageName) + '">'
+ text + '</tw-broken-link>';
return;
......
......@@ -59,6 +59,14 @@ define(['jquery', 'utils', 'utils/selectors'], ($, {unescape}, Selectors) => {
});
return ret.sort((left, right) => left.get('name') > right.get('name'));
},
/*
This provides more data-checking than the built-in "has()".
*/
hasValid(name) {
const passageData = this.get(name);
return passageData && (passageData instanceof Map) && passageData.has('source');
},
create: Passage,
});
......
......@@ -491,26 +491,29 @@ define(['utils', 'passages', 'datatypes/changercommand', 'internaltypes/twineerr
/*
Deserialise the string and replace the current history.
@method deserialise
Since an error with save data isn't necessarily an author error, the errors returned
by this function aren't TwineErrors.
*/
function deserialise(str) {
let newTimeline,
lastVariables = SystemVariables;
const genericError = "The save data is unintelligible.";
try {
newTimeline = JSON.parse(str, reviver);
}
catch(e) {
return false;
return Error(genericError);
}
/*
Verify that the timeline is an array.
*/
if (!Array.isArray(newTimeline)) {
return false;
return Error(genericError);
}
if ((newTimeline = newTimeline.map((moment) => {
let momentError;
if ((momentError = (newTimeline = newTimeline.map((moment) => {
/*
Here, we do some brief verification that the moments in the array are
objects with "passage" and "variables" keys.
......@@ -518,7 +521,14 @@ define(['utils', 'passages', 'datatypes/changercommand', 'internaltypes/twineerr
if (typeof moment !== "object"
|| !moment.hasOwnProperty("passage")
|| !moment.hasOwnProperty("variables")) {
return false;
return Error(genericError);
}
/*
Check that the passage name in this moment corresponds to a real passage.
As this is the most likely issue with invalid save data, this gets a precise message.
*/
if (!Passages.hasValid(moment.passage)) {
return Error("The data refers to a passage named '" + moment.passage + "', but it isn't in this story.");
}
/*
Recreate the variables prototype chain. This doesn't use setPrototypeOf() due to
......@@ -531,13 +541,14 @@ define(['utils', 'passages', 'datatypes/changercommand', 'internaltypes/twineerr
Re-establish the moment objects' prototype link to Moment.
*/
return Object.assign(Object.create(Moment), moment);
})).includes(false)) {
return false;
})).find(e => e instanceof Error))) {
return momentError;
}
timeline = newTimeline;
eventHandlers.load.forEach(fn => fn(timeline));
recent = timeline.length - 1;
newPresent(timeline[recent].passage);
return true;
}
return {
serialise: serialise,
......
......@@ -31,6 +31,8 @@ describe("browser sessions", function() {
}, 20);
});
// Currently I can't test whether invalid sessionStorage data results in being silently ignored in harlowe.js.
afterAll(function() {
sessionStorage.clear();
});
......
......@@ -57,6 +57,16 @@ describe("enchantment macros", function () {
done();
},400);
});
it("works with (link-reveal:) links", function(done) {
var p = runPassage("(enchant: ?link, (text-colour: '#800000'))(link-reveal: \"foo\")[bar]");
setTimeout(function() {
var enchantment = p.find('tw-link').parent();
expect(enchantment.css('color')).toMatch(/(?:#800000|rgb\(\s*128,\s*0,\s*0\s*\))/);
p.find('tw-link').click();
expect(p.text()).toBe('foobar');
done();
},400);
});
});
describe("enchanting ?Page", function() {
it("wraps the ?Page in a <tw-enchantment>", function(done) {
......
......@@ -140,6 +140,16 @@ describe("save macros", function() {
}, 20);
});
});
it("produces a user-friendly prompt() for deletion if the save data is invalid", function() {
runPassage("uno", "uno");
runPassage("dos", "dos");
runPassage("(savegame:'1')", "tres");
runPassage("quatro", "quatro");
deletePassage('dos');
spyOn(window,'prompt');
expect("(loadgame:'1')").not.markupToError();
expect(window.prompt.calls.count()).toBe(1);
});
});
});
......@@ -53,6 +53,9 @@ window.runPassage = function(text, name, tags) {
return $('tw-passage:last-of-type > tw-transition-container, tw-passage > tw-hook');
};
window.deletePassage = function(name) {
Passages.delete(name);
};
var combinations = new Set(function recur(prefix, arr) {
var ret = [];
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment