1.18 forge

[esotericist]

draft pull request, since i haven't actually tested this build (due to not having the current markdown manual on curseforge, for it to be pulled by the normal cursemaven stuff).

somehow, i seem to have accidentally linted large swaths of the codebase to a new style. i don't know how i did this. if you need me to fix that before merging, i can try to work that out.

aside from that, this was all primarily mechanical in nature: find broken old thing, find new equivalent, move it over.

the least trivial code change was in li.cil.tis3d.common.capabilities, which i ended up eliding a lot more boilerplate than i expected. i honestly have no idea if i did it right; it LOOKS like the infrared receiver is working correctly from my tests, but i have to admit to not understanding the full nuance of its behavior.

edit: images. 'cuz why not.

[fnuecke]

Whoops, I clicked somewhere I didn't want to when I really wanted to make the pipelines run :x

Would be great if you could revert the whitespace changes (as you noted on Discord), would make this a lot easier to review. Thanks!

[esotericist]

Whoops, I clicked somewhere I didn't want to when I really wanted to make the pipelines run :x

been there, done that in other projects XD easy to fix, tho. sadly, the tests can't succeed until after the markdown manual build is posted to curseforge.

Would be great if you could revert the whitespace changes (as you noted on Discord), would make this a lot easier to review. Thanks!

one cherry pick, manual edit, and force-push. coming... well, not right up. but soon. ish.

[esotericist]

looks like the erroneous linting was limited to a single file, so that didn't bring down the changeset all that much. from +276-226 to +229-192. but hey, it's smaller!

edit: btw i kept the two commits in place so you could a) better see the progression as i had if you want to examine the individual commits, and b) the first commit is the one that actually builds (if you publish markdown manual to mavenlocal first). if you want to squash it on merge i won't care :D

[esotericist]

apparently i've done something incredibly wrong and machines do not function correctly across save/load cycles.

i have no idea what's wrong or how to fix it, since this port was pretty much at the edge of my understanding to begin with. :(

edit: i'm assuming it's something to do with the extra bits of capabilities logic i wasn't sure sure what to do with. like a handful of extra nuts and bolts after reassembling an appliance.

[esotericist]

after some sleep i decided to do some more data gathering.

it looks like the actual problem is in nbt related changes; in the 1.17 version, a freshly placed casing (even without a controller or any modules) has quite a lot of nbt data according to /data get block <target>

while a casing placed in my 1.18 port has no nbt data at all. but i'm unclear on which aspects of the nbt manipulation i ported over have incorrect semantics at this point.

edit: forge discord indicates we now need SaveAdditional for block entities, but i'm having trouble translating this information to working code.

[esotericist]

okay. inserted the commit for the nbt save semantics change into the middle of the timeline, for ease of isolated review (and to again provide an easily accessed 'this actually builds' version if you want to try it locally before markdown manual is pushed to curseforge)

edit2: see followup post, this was a misreport on my PR. edit: still not perfect, it looks like something can go awry in the save/load cycle for actively running programs. things appear to retain state, but don't continue to behave correctly.

i tested with the example program for the display module:

MOV 16, ACC
LOOP:
SUB 1
MOV ACC LEFT
MOV ACC LEFT
MOV 0 LEFT
MOV 1 LEFT
MOV 28 LEFT
JNZ LOOP

this works correctly if allowed to run to completion, but if you save and quit partway through the sequence, on reloading the display fails to continue to update. in fact, the display ends up sufficiently wedged the machine must be power cycled before it begins behaving correctly again.

i assume something is getting desynced due to me failing to handle some other aspect of nbt.

[esotericist]

good news*

this is not my bug. i can confirm the behavior in tis-3d 1.17.1. i'll file a separate issue later, after i test and see if i can find how far back the issue goes (and see if i can determine if it affects other modules).

at first it looked like the behavior in 1.17.1 is slightly different, but it instead looks like restarting reality results in undefined behavior in the tis architecture :grinning:

[fnuecke]

Thanks so much for the port and the detective work! Will look into that bug.

[esotericist]

delighted to; as i mentioned before, it's possible i'm the most dedicated actual user of the mod (as opposed to the normal people who go 'this looks neat. ... wait, what. naw, i'll just get open computers'), so i want it working :grin:

anyway, as soon as markdown manual is on curseforge, i think we'll be good to go for final build testing. pretty much everything i've tried has worked fine outside of the execution module reset issue, although i can't be certain i haven't missed anything until i sit down to use it in production (and i'm still waiting on a couple other mods before i get to minecraft playing for real on 1.18, but at the rate things are going it won't be long).

[fnuecke]

Hahaha, happy to hear that :)

Fix to the reset on load bug has been pushed to the 1.16 and 1.17 branches, if you want to merge that / rebase onto that for testing.

[esotericist]

should be good to go.

i did find another bug with the infrared module that dates back to the 1.16.5 port. i'll file that later, i r ti urd.

i'm so glad you've worked with me for this process, though.

[fnuecke]

Awesome, will give it a test run later. Have a good rest :)

[esotericist]

green checkmark! victory is mine!

[fnuecke]

Did some in-game testing, looking good! Thanks again :)