Skip to content

Commit fbe7f21

Browse files
committed
Fixes machines becoming invisible when breaking or replacing multiblock parts
The Root Cause was MachineRenderState relying on identity-based lookups (IdentityHashMap in model rendering, IdMapper in network sync). However, StateHolder.setValue() only returns interned instances when called on an already-interned state. When non-interned states entered the system via deserialization or network sync, identity lookups failed, causing blocks to render nothing. The Fix: - MachineRenderState - Added intern() method that looks up the canonical state instance from StateDefinition.getPossibleStates(), and wired it into the CODEC via xmap() so deserialized states are always interned - MetaMachineBlockEntity - setRenderState() now validates the state belongs to the correct definition and interns it; onLoad() also validates since @persisted bypasses the setter. - MachineRenderStatePayload - Interns state before registry ID lookup during network write - MultiblockControllerMachine - onPartUnload() now immediately invalidates the structure when a part is removed, ensuring all remaining parts have their render state updated to unformed - MachineModel - Added fallback rendering that searches by property values when identity lookup fails, preventing invisible blocks if a non-interned state slips through and is mostly a failsafe
1 parent f25a233 commit fbe7f21

File tree

7 files changed

+105
-17
lines changed

7 files changed

+105
-17
lines changed

src/main/java/com/gregtechceu/gtceu/api/blockentity/MetaMachineBlockEntity.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import net.minecraft.world.level.block.entity.BlockEntity;
2626
import net.minecraft.world.level.block.entity.BlockEntityType;
2727
import net.minecraft.world.level.block.state.BlockState;
28+
import net.minecraft.world.level.block.state.properties.Property;
2829

2930
import lombok.Getter;
3031
import org.jetbrains.annotations.Nullable;
@@ -117,10 +118,50 @@ public void removeComponentsFromTag(CompoundTag tag) {
117118

118119
@Override
119120
public void setRenderState(MachineRenderState state) {
121+
if (state != null) {
122+
// Validate that the state belongs to this block entity's definition
123+
if (!state.is(getDefinition())) {
124+
state = correctRenderStateDefinition(state);
125+
}
126+
// Ensure we're using the interned (canonical) state instance
127+
state = state.intern();
128+
}
120129
this.renderState = state;
121130
scheduleRenderUpdate();
122131
}
123132

133+
/**
134+
* Corrects a render state that belongs to the wrong definition by creating a new state
135+
* from this block entity's definition while preserving any compatible property values.
136+
*/
137+
private MachineRenderState correctRenderStateDefinition(MachineRenderState wrongState) {
138+
MachineRenderState correctedState = getDefinition().defaultRenderState();
139+
// Copy any property values that exist in both the wrong state and this definition's state
140+
for (Property<?> property : wrongState.getProperties()) {
141+
if (correctedState.hasProperty(property)) {
142+
correctedState = copyProperty(correctedState, wrongState, property);
143+
}
144+
}
145+
return correctedState; // intern() will be called by setRenderState
146+
}
147+
148+
/**
149+
* Helper method to copy a single property value from one state to another.
150+
* Uses a type parameter to satisfy the generic constraints of setValue().
151+
*/
152+
@SuppressWarnings("unchecked")
153+
private static <T extends Comparable<T>> MachineRenderState copyProperty(
154+
MachineRenderState target,
155+
MachineRenderState source,
156+
Property<T> property) {
157+
T value = source.getValue(property);
158+
// Verify the value is valid for the target state's property
159+
if (property.getPossibleValues().contains(value)) {
160+
return target.setValue(property, value);
161+
}
162+
return target;
163+
}
164+
124165
@Override
125166
public long getOffset() {
126167
return offset;
@@ -135,6 +176,11 @@ public void setRemoved() {
135176
@Override
136177
public void onLoad() {
137178
super.onLoad();
179+
// Validate render state after NBT load - @Persisted writes directly to field via reflection,
180+
// bypassing setRenderState(), so we need to validate here as well
181+
if (renderState != null && !renderState.is(getDefinition())) {
182+
this.renderState = correctRenderStateDefinition(renderState);
183+
}
138184
metaMachine.onLoad();
139185
}
140186

src/main/java/com/gregtechceu/gtceu/api/machine/multiblock/MultiblockControllerMachine.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import net.minecraft.core.BlockPos;
2424
import net.minecraft.core.Direction;
2525
import net.minecraft.server.level.ServerLevel;
26-
import net.minecraft.world.level.block.state.BlockState;
2726

2827
import lombok.Getter;
2928
import lombok.Setter;
@@ -204,16 +203,25 @@ public void onStructureInvalid() {
204203
}
205204

206205
/**
207-
* mark multiblockState as unload error first.
208-
* if it's actually cuz by block breaking.
209-
* {@link #onStructureInvalid()} will be called from
210-
* {@link MultiblockState#onBlockStateChanged(BlockPos, BlockState)}
206+
* Called when a part is unloaded (chunk unload or block broken).
207+
* If the structure is currently formed, immediately invalidate it to ensure
208+
* all remaining parts have their render state updated.
211209
*/
212210
@Override
213211
public void onPartUnload() {
214-
parts.removeIf(part -> part.self().isInValid());
212+
for (var it = parts.iterator(); it.hasNext();) {
213+
var part = it.next();
214+
if (part.self().isInValid()) {
215+
it.remove();
216+
}
217+
}
215218
getMultiblockState().setError(MultiblockState.UNLOAD_ERROR);
216219
if (getLevel() instanceof ServerLevel serverLevel) {
220+
// If structure is formed, invalidate it immediately to update all parts' render states
221+
if (isFormed) {
222+
onStructureInvalid();
223+
MultiblockWorldSavedData.getOrCreate(serverLevel).removeMapping(getMultiblockState());
224+
}
217225
MultiblockWorldSavedData.getOrCreate(serverLevel).addAsyncLogic(this);
218226
}
219227
updatePartPositions();

src/main/java/com/gregtechceu/gtceu/api/multiblock/MultiblockState.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,6 @@ public void onBlockStateChanged(BlockPos pos, BlockState state) {
172172
}
173173
} else {
174174
IMultiController controller = getController();
175-
if (controller == null && error == UNLOAD_ERROR) {
176-
if (!serverLevel.isLoaded(controllerPos)) {
177-
GTCEu.LOGGER.info("Controller not loaded, pos {}", controllerPos);
178-
}
179-
}
180175
if (controller != null) {
181176
if (controller.isFormed() && state.getBlock() instanceof ActiveBlock) {
182177
LongSet activeBlocks = getMatchContext().getOrDefault("vaBlocks", LongSets.emptySet());
@@ -186,7 +181,8 @@ public void onBlockStateChanged(BlockPos pos, BlockState state) {
186181
return;
187182
}
188183
}
189-
if (controller.checkPatternWithLock()) {
184+
boolean patternValid = controller.checkPatternWithLock();
185+
if (patternValid) {
190186
// refresh structure
191187
controller.self().setFlipped(this.neededFlip);
192188
controller.onStructureFormed();

src/main/java/com/gregtechceu/gtceu/client/model/machine/MachineModel.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,22 @@ public void renderBaseModel(List<BakedQuad> quads, @NotNull MachineRenderState r
293293
}
294294
if (modelsByState.containsKey(renderState)) {
295295
quads.addAll(modelsByState.get(renderState).getQuads(blockState, side, rand, modelData, renderType));
296+
} else {
297+
// State not found by identity - try to find matching state by values as fallback
298+
MachineRenderState fallbackState = null;
299+
for (MachineRenderState expected : modelsByState.keySet()) {
300+
if (expected.getValues().equals(renderState.getValues())) {
301+
fallbackState = expected;
302+
break;
303+
}
304+
}
305+
if (fallbackState != null) {
306+
quads.addAll(modelsByState.get(fallbackState).getQuads(blockState, side, rand, modelData, renderType));
307+
} else if (modelsByState.containsKey(definition.defaultRenderState())) {
308+
// Fall back to default state as last resort
309+
quads.addAll(modelsByState.get(definition.defaultRenderState())
310+
.getQuads(blockState, side, rand, modelData, renderType));
311+
}
296312
}
297313
}
298314

src/main/java/com/gregtechceu/gtceu/client/model/machine/MachineRenderState.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,31 @@
1111
import com.mojang.serialization.MapCodec;
1212
import it.unimi.dsi.fastutil.objects.Reference2ObjectArrayMap;
1313

14+
import java.util.function.Function;
15+
1416
public class MachineRenderState extends StateHolder<MachineDefinition, MachineRenderState> {
1517

18+
/**
19+
* CODEC that always returns interned (canonical) state instances.
20+
* This is critical because StateHolder uses identity-based lookups internally.
21+
*/
1622
public static final Codec<MachineRenderState> CODEC = codec(GTRegistries.MACHINES.byNameCodec(),
17-
MachineDefinition::defaultRenderState).stable();
23+
MachineDefinition::defaultRenderState)
24+
.xmap(MachineRenderState::intern, Function.identity())
25+
.stable();
26+
27+
/**
28+
* Returns the interned (canonical) instance of this state from the definition's StateDefinition.
29+
* This ensures identity-based lookups work correctly.
30+
*/
31+
public MachineRenderState intern() {
32+
for (MachineRenderState interned : getDefinition().getStateDefinition().getPossibleStates()) {
33+
if (interned.getValues().equals(this.getValues())) {
34+
return interned;
35+
}
36+
}
37+
return this; // Fallback if not found (shouldn't happen)
38+
}
1839

1940
public MachineRenderState(MachineDefinition owner, Reference2ObjectArrayMap<Property<?>, Comparable<?>> values,
2041
MapCodec<MachineRenderState> propertiesCodec) {

src/main/java/com/gregtechceu/gtceu/data/machine/GTMachines.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import com.google.common.math.IntMath;
5050
import it.unimi.dsi.fastutil.Pair;
5151

52-
import java.util.Iterator;
5352
import java.util.List;
5453
import java.util.function.BiConsumer;
5554

@@ -1101,9 +1100,7 @@ public static void init() {
11011100
}
11021101

11031102
public static void bakeRenderStates(Registry<MachineDefinition> registry) {
1104-
Iterator<MachineDefinition> iter = registry.iterator();
1105-
while (iter.hasNext()) {
1106-
MachineDefinition machine = iter.next();
1103+
for (MachineDefinition machine : registry) {
11071104
for (MachineRenderState renderState : machine.getStateDefinition().getPossibleStates()) {
11081105
MachineDefinition.RENDER_STATE_REGISTRY.add(renderState);
11091106
}

src/main/java/com/gregtechceu/gtceu/syncdata/MachineRenderStatePayload.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ public class MachineRenderStatePayload extends ObjectTypedPayload<MachineRenderS
1818

1919
@Override
2020
public void writePayload(RegistryFriendlyByteBuf buf) {
21+
// Ensure we're using the interned state for registry lookup
22+
if (payload != null) {
23+
payload = payload.intern();
24+
}
2125
buf.writeById(MachineDefinition.RENDER_STATE_REGISTRY::getId, payload);
2226
}
2327

0 commit comments

Comments
 (0)