-
-
Notifications
You must be signed in to change notification settings - Fork 811
Add event for when a game save is updated #1094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Any performance impact to this? Do you have any fallback for if EJS is not being run on https? |
|
I didn't notice any performance impact (and it should be handled asynchronously, so I don't think it should matter). I don't have a fallback for not HTTPS, unfortunately—I briefly examined it, but I didn't want to do something overly complicated. Edit: And just to clarify a bit more, it seems like an own fallback would require an external library or writing our own algorithm. Subtle crypto is a native browser implementation, and so is likely also the most performant way to achieve this. |
|
In that case; can you do a check for https and bypass (return to existing behaviour) this code if it's running in plain http? |
|
I'm not sure what you mean. It's only adding a new event in the case where it can be compared by hash because of HTTPS/localhost—or are you saying to invoke their callback in the saveSave event (with a null hash)? |
|
Yup... null hash. Currently in Gaseous, I'm simply polling EJS every x seconds for the file and if the last updated datetime is different uploading it to my C# server. My C# server is then doing the hashing (no https required) to determine if the file is different. I do it this way because a) there's no way to tell if the file has changed or not (currently), and b) not everyone is running in https. |
|
I do think there should be some method of fallback here.. not sure how |
|
Comments on the review so far are all fine. Will think about a fallback. It's late here, so I'll submit a revision tomorrow. |
|
Switched to Cyrb53, per this StackOverflow question: https://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript. It also eliminates the dependency on subtle crypto. I did run a few tests with millions of random elements and did not encounter even a single collision: Good enough for not cryptographic purposes, anyway. Some performance notes on my M2 MacBook Air:
In other words, this hash should be fast enough for all cores flushing saves at any sensible frequency. If we were to poll even every 100ms on PPSSPP, the update callback would be delayed by only about 1% of that cycle time. (The bigger concern is how frequently the cores themselves can dump.) Rudimentary performance testing codevar testExponent = 3;
var randomStrings = []
var testCount = () => 10 ** testExponent
var encoder = new TextEncoder();
var generateRandomString = () => (Math.random()*10).toString(36).substring(4).repeat(80000);
for (let i = 0; i < testCount(); i++) {
randomStrings[i] = encoder.encode(generateRandomString()); // Since saves are buffers
}
console.log(`Preparing test of ${randomStrings.length} buffers of approximate size ${randomStrings[0].length}`)
// --- CYRB TEST---
var generateHashCyrb = async(str, seed = 0) => {
let h1 = 0xdeadbeef ^ seed, h2 = 0x41c6ce57 ^ seed;
for(let i = 0, ch; i < str.length; i++) {
ch = str[i];
h1 = Math.imul(h1 ^ ch, 2654435761);
h2 = Math.imul(h2 ^ ch, 1597334677);
}
h1 = Math.imul(h1 ^ (h1 >>> 16), 2246822507);
h1 ^= Math.imul(h2 ^ (h2 >>> 13), 3266489909);
h2 = Math.imul(h2 ^ (h2 >>> 16), 2246822507);
h2 ^= Math.imul(h1 ^ (h1 >>> 13), 3266489909);
return 4294967296 * (2097151 & h2) + (h1 >>> 0);
};
var startTime = performance.now();
await (async () => {for (i = 0; i < testCount(); i++) {
(await generateHashCyrb(randomStrings[i])).toString(16);
}})()
var endTime = performance.now();
console.log(`cyrb53 test done, took ${endTime - startTime} milliseconds`);
// --- SHA TEST ---
function digestAsHexString(buffer) {
const hashArray = Array.from(new Uint8Array(buffer));
return hashArray.map((b) => b.toString(16).padStart(2, "0")).join("");
}
var startTime = performance.now();
await (async () => {for (i = 0; i < testCount(); i++) {
await window.crypto.subtle.digest("SHA-1", randomStrings[i]).then(digestAsHexString);
}})()
var endTime = performance.now();
console.log(`SHA-1 test done, took ${endTime - startTime} milliseconds`); |
ethanaobrien
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think this looks good, thank you
|
Looks good... happy to approve once documentation changes have been proposed :) |
|
Proposed a bit of documentation |
|
Awesome... thanks @cryptonaus! |
Adds an event,
saveUpdatethat triggers when 1) a new save occurs and 2) the hash of the new save is new or differs from the prior hash. The dependency on subtle crypto for hashing necessitates a secure context to use this functionality.Combined with #1095, can provide a pretty reliable save update event. I tested with 80ms and had no issues.