Thermostat Modules don't appear to be updating
MuteTiefling opened this issue ยท 12 comments
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.
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
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
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.
@johalun any thoughts? I'd take a look myself but probably no chance until the weekend
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.
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.
@MuteTiefling Thanks for the detailed bug report ๐๐ป It made it very easy to track down the issue.
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() {
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.
This appears to still be broken on servers in 1.19.2. The thermostat works okay in single player, though.
Ignore this. User error.