MightyPirates / TIS-3D

TIS-100 inspired low-tech computing in Minecraft.
https://www.curseforge.com/minecraft/mc-mods/tis-3d
Other
105 stars 35 forks source link

execution modules have their state improperly saved across save/load cycles #149

Closed esotericist closed 2 years ago

esotericist commented 2 years ago

this originally showed up while i was working on #148 and trying to test that i hadn't broken anything else fiddly.

what i saw was that display modules, running a variant of the very simple example code for said display module, would have strange graphical problems across save/load cycles.

most obvious form is: 2021-12-06_02 54 28

the code backing that sequence being

MOV 16, ACC
LOOP:
SUB 1
MOV ACC LEFT
MOV ACC LEFT
MOV 0 LEFT
MOV 1 LEFT
SAV
ADD 9
MOV ACC LEFT
SWP
JNZ LOOP

at first this looked to me like data transmissions to the display module were somehow being dropped, and the sequencing was getting disordered. turns out i was only half right.

in order to get a stronger sense of the temporal aspect, i switched to using a terminal module. the hypothesis being: if data is being dropped, and the problem is not unique to the display module, i should see discontinuities in the terminal output.

the tested code:

MOV 0, ACC
LOOP:
SAV
ADD 48
MOV ACC LEFT
SUB 57
JLZ CONT
MOV 0, ACC
JMP LOOP
CONT:
SWP
ADD 1
JMP LOOP

when all is working properly, this should (and does, normally) result in a 0123456789 repeating sequence.

the experienced result: 2021-12-06_02 53 49

twice i saved and quit to the main menu, then loaded the game again. the result is pretty clear: the sequence is restarting from 0 from a save/load.

this issue is absent in the 1.12.2 version, but has been present in each major release starting from 1.16.5 (although i have to admit i only tested the most recent 1.16.5 version).

the above screenshots, btw, were in fact with the current 1.16.5 release on curseforge, but the behavior is identical on 1.17.1 and my work in progress 1.18 port.

i would have tested between 1.12.2 and 1.16.5, but those only had fabric releases, and it seemed like the 1.16.5 port had little code heritage in common with those branches.

back to the display module: the erratic behavior is actually due to it correctly maintaining state, and the execution module being the one to get desynced in which part of the sequence it is in.

so i'm pretty sure the issue here is isolated to the execution module.

fnuecke commented 2 years ago

Thanks again for investigating this. Am able to repro and found the cause. In short, the issue is that upon load, the execution module recompiles -- which also resets it. After it loaded its state. So the execution modules just always restart after loading. Looking at git blame I can't tell why this ever worked, I'll do some digging :x

fnuecke commented 2 years ago

Ah, turns out in the past (up to 1.12) the compile error was serialized. Now it's recomputed upon load. By recompiling. Which does the reset. Big whoops.

fnuecke commented 2 years ago

Fix pushed to 1.16 and 1.17 branches, thanks again for finding and helping track down the bug!