mirrors
/
CC-Tweaked
mirror of https://github.com/SquidDev-CC/CC-Tweaked
1
0
Fork 0
Code Issues Releases Wiki Activity

Don't cache the client monitor 

When rendering non-origin monitors, we would fetch the origin monitor,
read its client state, and then cache that on the current monitor to
avoid repeated lookups.

However, if the origin monitor is unloaded/removed on the client, and
then loaded agin, this cache will be not be invalidated, causing us to
render both the old and new monitor!

I think the correct thing to do here is cache the origin monitor. This
allows us to check when the origin monitor has been removed, and
invalidate the cache if needed.

However, I'm wary of any other edge cases here, so for now we do
something much simpler, and remove the cache entirely. This does mean
that monitors now need to perform extra block entity lookups, but the
performance cost doesn't appear to be too bad.

Fixes #1741
This commit is contained in:
Jonathan Coates 2024-03-10 10:57:56 +00:00
parent 451a2593ce
commit a9191a4d4e
No known key found for this signature in database
GPG Key ID: B9E431FF07C98D06
2 changed files with 23 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
projects/common/src/client/java/dan200/computercraft/client/render/monitor/MonitorBlockEntityRenderer.java
View File

@ -58,9 +58,9 @@ public MonitorBlockEntityRenderer(BlockEntityRendererProvider.Context context) {
@Override
public void render(MonitorBlockEntity monitor, float partialTicks, PoseStack transform, MultiBufferSource bufferSource, int lightmapCoord, int overlayLight) {
// Render from the origin monitor
var originTerminal = monitor.getClientMonitor();
var originTerminal = monitor.getOriginClientMonitor();
if (originTerminal == null) return;
var origin = originTerminal.getOrigin();
var renderState = originTerminal.getRenderState(MonitorRenderState::new);
var monitorPos = monitor.getBlockPos();

42
projects/common/src/main/java/dan200/computercraft/shared/peripheral/monitor/MonitorBlockEntity.java
View File

@ -45,13 +45,18 @@ public class MonitorBlockEntity extends BlockEntity {
private final boolean advanced;
private @Nullable ServerMonitor serverMonitor;
/**
* The monitor's state on the client. This is defined iff we're the origin monitor
* ({@code xIndex == 0 && yIndex == 0}).
*/
private @Nullable ClientMonitor clientMonitor;
private @Nullable MonitorPeripheral peripheral;
private final Set<IComputerAccess> computers = Collections.newSetFromMap(new ConcurrentHashMap<>());
private boolean needsUpdate = false;
private boolean needsValidating = false;
private boolean destroyed = false;
// MonitorWatcher state.
boolean enqueued;
@ -90,7 +95,7 @@ void destroy() {
@Override
public void setRemoved() {
super.setRemoved();
if (clientMonitor != null && xIndex == 0 && yIndex == 0) clientMonitor.destroy();
if (clientMonitor != null) clientMonitor.destroy();
}
@Override
@ -144,7 +149,7 @@ public ServerMonitor getCachedServerMonitor() {
private ServerMonitor getServerMonitor() {
if (serverMonitor != null) return serverMonitor;
var origin = getOrigin().getMonitor();
var origin = getOrigin();
if (origin == null) return null;
return serverMonitor = origin.serverMonitor;
@ -183,13 +188,11 @@ private void createServerTerminal() {
}
@Nullable
public ClientMonitor getClientMonitor() {
public ClientMonitor getOriginClientMonitor() {
if (clientMonitor != null) return clientMonitor;
var te = level.getBlockEntity(toWorldPos(0, 0));
if (!(te instanceof MonitorBlockEntity monitor)) return null;
return clientMonitor = monitor.clientMonitor;
var origin = getOrigin();
return origin == null ? null : origin.clientMonitor;
}
// Networking stuff
@ -210,17 +213,14 @@ public final CompoundTag getUpdateTag() {
}
private void onClientLoad(int oldXIndex, int oldYIndex) {
if (oldXIndex != xIndex || oldYIndex != yIndex) {
// If our index has changed then it's possible the origin monitor has changed. Thus
// we'll clear our cache. If we're the origin then we'll need to remove the glList as well.
if (oldXIndex == 0 && oldYIndex == 0 && clientMonitor != null) clientMonitor.destroy();
if ((oldXIndex != xIndex || oldYIndex != yIndex) && clientMonitor != null) {
// If our index has changed, and we were the origin, then destroy the current monitor.
clientMonitor.destroy();
clientMonitor = null;
}
if (xIndex == 0 && yIndex == 0) {
// If we're the origin terminal then create it.
if (clientMonitor == null) clientMonitor = new ClientMonitor(this);
}
// If we're the origin terminal then create it.
if (xIndex == 0 && yIndex == 0 && clientMonitor == null) clientMonitor = new ClientMonitor(this);
}
public final void read(TerminalState state) {
@ -287,7 +287,7 @@ public int getYIndex() {
}
boolean isCompatible(MonitorBlockEntity other) {
return !other.destroyed && advanced == other.advanced && getOrientation() == other.getOrientation() && getDirection() == other.getDirection();
return advanced == other.advanced && getOrientation() == other.getOrientation() && getDirection() == other.getDirection();
}
/**
@ -310,8 +310,8 @@ private MonitorState getLoadedMonitor(int x, int y) {
return isCompatible(monitor) ? MonitorState.present(monitor) : MonitorState.MISSING;
}
private MonitorState getOrigin() {
return getLoadedMonitor(0, 0);
private @Nullable MonitorBlockEntity getOrigin() {
return getLoadedMonitor(0, 0).getMonitor();
}
/**
@ -390,7 +390,7 @@ void updateNeighborsDeferred() {
}
void expand() {
var monitor = getOrigin().getMonitor();
var monitor = getOrigin();
if (monitor != null && monitor.xIndex == 0 && monitor.yIndex == 0) new Expander(monitor).expand();
}
@ -560,7 +560,7 @@ private boolean checkInvariants() {
}
var hasPeripheral = false;
var origin = getOrigin().getMonitor();
var origin = getOrigin();
var serverMonitor = origin != null ? origin.serverMonitor : this.serverMonitor;
for (var x = 0; x < width; x++) {
for (var y = 0; y < height; y++) {