Skip to content

Commit

Permalink
Add feature that warns about DataWatcher ID conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
makamys committed Nov 15, 2023
1 parent 390a8fd commit 64112b8
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/main/java/makamys/coretweaks/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ public class Config {
public static FeatureSetting coreTweaksCommand = FeatureSetting.FALSE;
@ConfigWrappedEnum(cat="Diagnostics", def=TRUE, com="Makes MapStorage's errors more informative.")
public static FeatureSetting enhanceMapStorageErrors;
@ConfigWrappedEnum(cat="Diagnostics", def=TRUE, com="Emit warning when a mod registers a typed DataWatcher object in an already occupied ID slot (vanilla only warns in the typeless registration method).")
public static FeatureSetting detectDataWatcherIdConflicts;
@ConfigBoolean(cat="Diagnostics.detect_data_watcher_id_conflicts", def=false, com="Keep track of all registration stack traces, and print which ones conflict. Off by default because it adds some overhead to DataWatcher object registration.")
public static boolean detectDataWatcherIdConflictCulprit;

//@ConfigBoolean(cat="Optimizations", def=FALSE, com="Use multi-threaded texture loading when stitching textures? Placebo.")
public static FeatureSetting threadedTextureLoader = FeatureSetting.FALSE;
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/makamys/coretweaks/MixinConfigPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ public List<String> getMixins() {
if(Config.enhanceMapStorageErrors.isActive()) {
mixins.add("diagnostics.enhancemapstorageerrors.MixinMapStorage");
}
if(Config.detectDataWatcherIdConflicts.isActive()) {
mixins.add("diagnostics.detectdatawatcherconflict.MixinDataWatcher");
}
}
return mixins;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package makamys.coretweaks.diagnostics;

import static makamys.coretweaks.CoreTweaks.LOGGER;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.WeakHashMap;

import org.apache.commons.lang3.exception.ExceptionUtils;

import net.minecraft.entity.DataWatcher;

public class DataWatcherMonitor {
private static WeakHashMap<DataWatcher, Map<Integer, List<AdditionRecord>>> data = new WeakHashMap<>();

public static void onAddition(DataWatcher dw, String entityClassName, int id) {
AdditionRecord record = new AdditionRecord();
List<AdditionRecord> recordsForId = data.computeIfAbsent(dw, k -> new HashMap<>()).computeIfAbsent(id, k -> new ArrayList<>());
recordsForId.add(record);

if(recordsForId.size() > 1) {
LOGGER.warn("Detected DataWatcher ID conflict at ID " + id + " for entity " + entityClassName);
for(int i = 0; i < recordsForId.size(); i++) {
LOGGER.warn("Stack trace for registration #" + (i + 1) + " (out of " + recordsForId.size() + "): ");
LOGGER.warn(recordsForId.get(i).stackTrace);
}
}
}

public static class AdditionRecord {
public final String stackTrace;
public AdditionRecord() {
stackTrace = ExceptionUtils.getStackTrace(new Throwable());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package makamys.coretweaks.mixin.diagnostics.detectdatawatcherconflict;

import static makamys.coretweaks.CoreTweaks.LOGGER;

import java.util.Map;

import org.apache.commons.lang3.exception.ExceptionUtils;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

import makamys.coretweaks.Config;
import makamys.coretweaks.diagnostics.DataWatcherMonitor;
import net.minecraft.entity.DataWatcher;
import net.minecraft.entity.Entity;

@Mixin(DataWatcher.class)
public class MixinDataWatcher {
@Shadow
private Entity field_151511_a;
@Shadow
private Map watchedObjects;

@Inject(method = "addObject", at = @At("HEAD"))
public void monitorObjectAddition(int id, Object object, CallbackInfo ci) {
if(Config.detectDataWatcherIdConflictCulprit && field_151511_a != null) {
DataWatcherMonitor.onAddition((DataWatcher)(Object)this, field_151511_a.getClass().getName(), id);
}
}

@Inject(method = "addObjectByDataType", at = @At("HEAD"))
public void monitorTypedObjectAddition(int id, int type, CallbackInfo ci) {
boolean printedCulprit = false;
if(Config.detectDataWatcherIdConflictCulprit && field_151511_a != null) {
printedCulprit = true;
DataWatcherMonitor.onAddition((DataWatcher)(Object)this, field_151511_a.getClass().getName(), id);
}
if(watchedObjects.containsKey(id)) {
LOGGER.warn("Detected duplicate DataWatcher object registration for entity " + (field_151511_a == null ? "null" : field_151511_a.getClass().getName()) + " at already occupied ID " + id + ". Things are likely going to break!" + (!Config.detectDataWatcherIdConflictCulprit ? " Enable `detectDataWatcherIdConflictCulprit` to gather more information." : ""));
if(!printedCulprit) LOGGER.warn("Last registration stack trace:\n" + ExceptionUtils.getStackTrace(new Throwable()));
}
}
}

0 comments on commit 64112b8

Please sign in to comment.