PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

43M Downloads

Thermostat Modules don't appear to be updating

MuteTiefling opened this issue ยท 10 comments

commented

Describe the bug

Thermostat's constantly read the heat value of the block they're pointing at as of the last block update they received.

How to reproduce the bug

For example, I set one to have a threshold of 1000 (block update when setting the value) as it was facing a TPP at 401 C. The TPP is now over 1000 C but the Thermostat hasn't updated and isn't outputting the expected redstone level.
2024-01-01_11 27 07
2024-01-01_11 27 08

Expected behavior

Thermostat should be updating as the attached block's heat changes

Additional details

Adding a timer next to the setup allows it to work. In the pictured setup, it's getting a block update every second from the Aura Vaporizer on the left
2024-01-01_11 34 27

Which Minecraft version are you using?

1.19

Which version of PneumaticCraft: Repressurized are you using?

pneumaticcraft-repressurized-1.19.2-4.3.13-47.jar

Crash log

No response

commented

Can you test out a dev build: https://modmaven.dev/me/desht/pneumaticcraft/pneumaticcraft-repressurized/6.0.13+mc1.20.1-SNAPSHOT/pneumaticcraft-repressurized-6.0.13+mc1.20.1-20240105.092446-4.jar

Should resolve the issue. Be sure to test it continues to work if the module and/or monitored block are broken/replaced.

commented

Looks to be working flawlessly ๐Ÿ‘๐Ÿป

commented

I dropped the dev build into a Valhelsia 6 instance.

commented

@johalun any thoughts? I'd take a look myself but probably no chance until the weekend

commented

I guess this means that the block that's being measured does not trigger a neighbor block update. What would be the best way to update the thermostat without external block update? tickServer feels excessive.

commented

Looking at the RedstoneModule it does update on every tick. Maybe just calling updateInputLevel unconditionally on each server tick is the way to go? It only send network packets if the output redstone signal changes so the per tick CPU load is minimal I think, unless those getters for world, pos and heatexchangemanger are heavy.

commented

@MuteTiefling Thanks for the detailed bug report ๐Ÿ‘๐Ÿป It made it very easy to track down the issue.

commented

This change (on 1.20.1) would cause update on every server tick, but still only send update to clients if there's a change in redstone output. I can create PRs if you think this is acceptable @desht
Tested by heating up an iron block with a vortex tube. The thermostat and the vortex tube being on opposite sides of the iron block so it should not get block update from the vortex tube I think.

diff --git a/src/main/java/me/desht/pneumaticcraft/common/network/PacketSyncThermostatModuleToServer.java b/src/main/java/me/desht/pneumaticcraft/common/network/PacketSyncThermostatModuleToServer.java
index 142db2eab..046fdd18f 100644
--- a/src/main/java/me/desht/pneumaticcraft/common/network/PacketSyncThermostatModuleToServer.java
+++ b/src/main/java/me/desht/pneumaticcraft/common/network/PacketSyncThermostatModuleToServer.java
@@ -68,7 +68,6 @@ public class PacketSyncThermostatModuleToServer extends LocationIntPacket {
                         mr.setColorChannel(channel);
                         mr.setThreshold(threshold);
                         mr.updateNeighbors();
-                        mr.setUpdate(true); // Force recalc
                     }
                 });
             }
diff --git a/src/main/java/me/desht/pneumaticcraft/common/tubemodules/ThermostatModule.java b/src/main/java/me/desht/pneumaticcraft/common/tubemodules/ThermostatModule.java
index fd309625c..972926732 100644
--- a/src/main/java/me/desht/pneumaticcraft/common/tubemodules/ThermostatModule.java
+++ b/src/main/java/me/desht/pneumaticcraft/common/tubemodules/ThermostatModule.java
@@ -47,7 +47,6 @@ public class ThermostatModule extends AbstractTubeModule implements INetworkedMo
     private int temperature = 0;
     private int level;
     private int threshold;
-    private boolean update = true;

     public ThermostatModule(Direction dir, PressureTubeBlockEntity pressureTube) {
         super(dir, pressureTube);
@@ -86,10 +85,6 @@ public class ThermostatModule extends AbstractTubeModule implements INetworkedMo
         this.level = level;
     }

-    public void setUpdate(boolean update) {
-        this.update = update;
-    }
-
     @Override
     public boolean hasGui() {
         return true;
@@ -169,12 +164,7 @@ public class ThermostatModule extends AbstractTubeModule implements INetworkedMo
     @Override
     public void tickServer() {
         super.tickServer();
-
-        // Forced recalc when client GUI updated
-        if (this.update) {
-            this.update = false;
-            updateInputLevel();
-        }
+        updateInputLevel();
     }

     public void updateInputLevel() {
commented

Looking at the RedstoneModule it does update on every tick

The input mode doesn't, though - only on the first tick (or when inputLevel < 0, which indicates a cached signal is invalidated)

I don't think calling updateInputLevel() every level is ideal. It's not the most expensive operation ever, but it does add up. Checking the code, there isn't any specific notification for when the temperature changes on the side of a block.

I have an idea though, a bit more API to allow objects to register a listener on heat exchangers, to be called when the temperature changes. I'll push a commit later on today.

commented

This appears to still be broken on servers in 1.19.2. The thermostat works okay in single player, though.

Ignore this. User error.