Redstone pen block update orientation bug
lukescott opened this issue · 10 comments
There is a block update bug with the red stone path created with the pen. Hard to describe, easier to demonstrate:
rs_path_bug_demo.mp4
It seems the orientation matters, N/S vs E/W, and also the behavior differs a bit from shown if I flip the design (torch on right instead of left and block inverted).
Three distinct behaviors though, (1) connecting blocks don't power until a block update, (2) connecting blocks don't unpower until block update, and (3) differing behavior when you draw a line with one click vs two clicks, and erasing either side.
Connecting full line:
Note only single block lights and the adjacent do not unless a manual block update is performed by placing adjacent block.
Disconnect half line to block, reconnect, then disconnect half line to torch:
Outer blocks are still powered with center unpowered, the outer blocks do not unpower unless you block update the adjacent block.
Player facing the sun (day/night disabled, so assuming morning).
All of this is happening within the same chunk.
Minecraft 1.20.4
Mod Version: 1.8.26
Loader: Fabric
There is also an issue with vanilla Redstone picking up power thru a block provided by the mod's Redstone track. Although I assume the answer is you shouldn't mix the two. (And it seemed like a separate issue).
Hi Luke, thanks for the detailed report and sorry about the late response - I am back home now ;).
Let me check that, this looks indeed like a missing redstone-neighbour-changed update when the wire network is changed and the powering branch removed.
As redstone changes little but strangely in vanilla, messing up the optimizations of the Redstone Pen Track networks, which
mod file did you use? (or Minecraft version / mod version / fabric or forge).
Hey luke, I pushed an update to curse (as beta, initial tests ok, but you never know ;) ).
It's in review now and should be public in a few hours: https://legacy.curseforge.com/minecraft/mc-mods/redstone-pen/files/5012166
The redstone track change detection was messed up (the neighbour notifications for removed or added track segments).
The fix adds a bit of performance overhead, but that should be ok, as it's only calculated when redstone is added or removed in a Track block. The "normal operation" is still optimised.
Could you risk a glimpse if the problem is also resolved in your setup? Cheers;
Hi, let's start with the last one first: Yes, the track applies strong power to the lamp, and the lamp does not insulate like glass, so weak power is transferred to the vanilla wire. Torch powers the track with 15, track the lamp with 15 (no wire or track means not reducing the redstone signal), wire 15 from the lamp. The lamp underneath the wire gets 15 and the adjacent blocks, too (vanilla wire applies strong power).
The catch is the the vanilla wire checks if the indirect power comes from a vanilla wire or not to ignore the power. All other signals are accepted. E.g. placing a RS torch under the lamp:
The quirk should only work one indirect hop, as the track will ignore the indirect vanilla wire signal, and a wire would also do so.
For actually fixing that, hooking into the vanilla wires would be needed (only not updating the wire from the track is not enough, because other block updates may trigger that the vanilla wire recalculates). I refrain from that, because other redstone mods like Alternate Current do that already, bugs arising from that would be very hard to trace. So - let's live with the glitch until a change in MC opens a way to circumvent the problem in a unified way.
... I answer the other stuff in the separate comment, just saw that
Direction has no translation codec anymore, which causes exception
messages in the log ...
Looks like the fix works. I don't really have a setup per say, I was testing in various scenarios to figure out how Redstone works in general, and how the mod differs from vanilla (I've played Minecraft before, just never delved into the Redstone as much).
I reviewed the commit. I have a question: Before the fix, 1 out of 4 orientations worked correctly, where the other 3 of 4 suffered from one or both connection/disconnection issues. Why would one work, and the others not?
I was expecting some sort of orientation fix to the existing code since it seemed to work in at least one of the cases. I wasn't expecting a new code block. Could you provide some context?
There is a lot happening in updateConnections function, perhaps it would be worth breaking it apart into smaller sections? I do have a software engineering background, so I could potentially help with that. I just don't have Minecraft modding experience. (Still reading thru some code and wrapping my head around some of the concepts).
I found this interesting quirk, and it could totally be working as intended, but thought I'd mention:
Redstone track left, vanilla Redstone right. The vanilla Redstone shouldn't be powered here. If you do vanilla left/right and track left/right the right side isn't powered as expected. Does the track work like a repeater where it sends power thru the block? (That mechanic with Redstone is so confusing; Even w/ vanilla redstone on the left, a repeater on the right will pull power).
About the current fix: The main bug was that no explicit redstone notifications were invoked on the blocks in reach. The update order still needs consideration, IIRC I switched from a list to map, which may not preserve the insertion order, and from a short view MC has changed by adding the NeighbourUpdater. So I changed stuff to LinkedHashMap() and explicitly copied the update order from MC again, but testing the exact effects is unfortunately out of scope now. So it's not yet the end of the fixing line. But out of time for today ;)
About updateConnections(): Has simply grown with porting and features. It's still bearable for a hobby project without unit testing ;).
As for in general about modding: The actual challenge - at least for hobby modders - is to keep up with the MC releases, and the different handling of forge, fabric, and now also neoforge. With each new MC version the API and other interior change without documentation. It's a fun challenge, but that also eats up quite a slice of the time that one has for hobbies. Hence, I don't consider/expect it to be professional code with good architecture and ci testing - just fun coding to keep in touch with Java and have some nicer RS handling in MC ;)
Cheers ,-
The catch is the the vanilla wire checks if the indirect power comes from a vanilla wire or not to ignore the power. All other signals are accepted.
I figured it was probably working as intended 👍 . But thought I would check just in case. So essentially you'd have to override functionality to the vanilla redstone to add your track to that check as an additional exception. And depending on how deep that is buried, you'd have to re-implement code, which could cause issues, in accurate reproduction, but also down the line when Mojang updates it themselves.
I do find vanilla redstone confusing sometimes anyway. I sometimes wish someone would do a "redstone 2.0" that disregarded the original redstone and worked independently. This mod is probably the closest thing to that 😄 .
About the current fix: The main bug was that no explicit redstone notifications were invoked on the blocks in reach. The update order still needs consideration, IIRC I switched from a list to map, which may not preserve the insertion order, and from a short view MC has changed by adding the NeighbourUpdater. So I changed stuff to LinkedHashMap() and explicitly copied the update order from MC again, but testing the exact effects is unfortunately out of scope now. So it's not yet the end of the fixing line. But out of time for today ;)
About updateConnections(): Has simply grown with porting and features. It's still bearable for a hobby project without unit testing ;).
As for in general about modding: The actual challenge - at least for hobby modders - is to keep up with the MC releases, and the different handling of forge, fabric, and now also neoforge. With each new MC version the API and other interior change without documentation. It's a fun challenge, but that also eats up quite a slice of the time that one has for hobbies. Hence, I don't consider/expect it to be professional code with good architecture and ci testing - just fun coding to keep in touch with Java and have some nicer RS handling in MC ;)
Got it. Thank you for your hard work in this.
The second commit seems closer to what I thought it would be - having something to do with direction. Assuming MC's update order probably is fixed to a cardinal direction, so when model rotated, that update order had an effect.
I'm still not sure how the first commit and second commit connect as a lack all the knowledge in-between. Although I somewhat get the gist of them independently. I'll get there, just going to take some time.
Just a curious question: Is unit testing something that's possible with Minecraft modding? I imagine it would be hard without being able to test against the actual mechanics of the game. So you'd likely only be able to test discrete units that don't interact with MC directly.
Aye, Forge/Neoforge provides test runs for gradle. I think to have seen it also in the fabric build conf. The thing is, unit testable are mostly internal module logic, while the most part of mods needs integration testing and UI-like testing (at least to my brain). In the past, problems mostly came from dependency related issues, mod cross-compatibility, and me misinterpreting/incorrectly guessing MC functionality that I did override or use. So, mod testing is likely a more explorative thing.
E.g. I did unit testing for the descending math expression parser of the RLC in a separate java project, and integrated it then in the mod. I think it is unlikely that changes in java will invalidate that code while still compiling. That was one of the well-testable things ;)
Cheers,-