Detect a class of race conditions in which the game state has changed between load and save

[cgolubi1]

• Fixes Die reappeared after being captured; a different one disappeared #2780

[cgolubi1]

I can do some replay testing on this, but first i want Shadowshade's take on whether my argument in #2780 makes sense, and whether this seems like it would help (or at least not hurt).

[blackshadowshade]

Something about this seems wrong to me. I can't exactly put my finger on it, but adding an extra load in the save logic may well hurt, since it slows down the save. Also, using an assert is almost certainly the wrong way to deal with this error.

Let me take another look when it's not so hot and I can actually think about the problem some more.

[cgolubi1]

Sounds good. For what it's worth, my expectation was that the catch block would catch the assert and roll back the transaction. If that's not the way asserts work in PHP, then i agree we should throw a different type of exception --- that's just a one-line change, not fundamental to the idea.

Anyway, let me know what you think --- i agree there's no rush on this one.

[blackshadowshade]

Okay, I've had another look at this, and I think I know which part seems wrong to me.

Why would we use the game state when we could use the last_action_timestamp?

The problem with using game state is that it does not necessarily change after an action takes place. However, the last_action_timestamp should.

Does that sound right to you?

[cgolubi1]

Ah, okay. I didn't think of using last_action_timestamp; i agree i can't think of a reason it wouldn't be preferable. I'll try it out!