BUGFIX: Fix #795 #1231
BUGFIX: Fix #795 #1231
Conversation
|
I think the only difference here is how time skipping affects scripts. By just changing the last update time with no reload, most game mechanics should accumulate bonus time and function identically to if we were actually offline for X amount of time, but scripts will be unaffected since they don't rely on a last update time at all. Whereas with the current reload approach, scripts should function as if they were offline for X amount of time as well. Still, there's not really a right or wrong way for this to work, and I don't really like the reload thing, so I'm down with merging this change. Alternately we could make both options available, maybe keeping a timeskip+reload option is helpful to test offline impact of scripts. |
|
If we want to implement the (reload+offline for X), we need to change time skip code a little. In the next engine loop, the "backward" last update time will be overwritten with the current timestamp. If the auto save (which may occur due to the "backward" last update time) and the reload action happen after that, the offline time (after reloading) will be nearly 0. It's a bit troublesome to avoid this problem. For example, we can I can implement a timeskip+reload option if there is demand for it; otherwise, I will keep this PR simple with current approach. |
|
We might want something to make testing of offline time easier, but right now this doesn't do it (despite seeming like it does). There's a usecase for both, but IMO it's best to have this working the simple way (dealing with reloads is also annoying as a user) and then we can assess if we need an offline timeskip version. |
I don't think you even would need to stop the engine loop, since the reload should be synchronous? |
|
The engine loop may still run after the call of This code shows that small window: <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>test-tools</title>
<link rel="shortcut icon" href="data:," />
</head>
<body>
<script>
console.log("loaded");
const random = Math.random();
const engineLoop = () => {
console.log(random, Date.now());
window.setTimeout(engineLoop, 200);
};
console.log("start engine loop");
engineLoop();
setTimeout(() => {
console.log("reload");
location.reload();
}, 2000);
</script>
</body>
</html>Sample log: With the "delay" of 200, this rarely happens. |
This is the follow-up PR for #1223.
@d0sboots's PR fixes the root cause of #795, but there was a concern about the misleading popup after we reload. "Save+reload" is unnecessary now because the effect will be applied in the next engine loop. This PR replaces the "Save+reload" code with a message dialog.
Fixes #795.