fix(ServerDataSyncer): reset quires correctly if op level changed

src/main/java/fi/dy/masa/tweakeroo/config/FeatureToggle.java

@@ -36,6 +36,7 @@ public enum FeatureToggle implements IHotkeyTogglable, IConfigNotifiable<IConfig
     //TWEAK_CUSTOM_FLAT_PRESETS       ("tweakCustomFlatPresets",              false, "",    "Allows adding custom flat world presets to the list.\nThe presets are defined in Lists -> flatWorldPresets"),
     TWEAK_CUSTOM_FLY_DECELERATION   ("tweakCustomFlyDeceleration",          false, "",    "Allows changing the fly deceleration in creative or spectator mode.\nThis is mainly meant for faster deceleration ie. less \"glide\"\nwhen releasing the movement keys.\nSee Generic -> flyDecelerationRampValue"),
     TWEAK_CUSTOM_INVENTORY_GUI_SCALE("tweakCustomInventoryScreenScale",     false, "",    "Allows using a custom GUI scale for any inventory screen.\nSee Generic -> §ecustomInventoryGuiScale§r for the scale value"),
+    TWEAK_DISABLE_SERVER_DATA_SYNC  ("tweakDisableServerDataSync",          false, "",    "Disables Server Data Syncer for entities such as Shulker Boxes,\nmaking inventoryPreview cannot render correct data on servers.\nYou must be an operator of the server or install some server-side\nmods to make the syncer work even this option is false."),
     TWEAK_ELYTRA_CAMERA             ("tweakElytraCamera",                   false, "",    "Allows locking the real player rotations while holding the 'elytraCamera' activation key.\nThe controls will then only affect the separate 'camera rotations' for the rendering/camera.\nMeant for things like looking down/around while elytra flying nice and straight."),
     TWEAK_ENTITY_REACH_OVERRIDE      ("tweakEntityReachOverride",           false, true, "",    "Overrides the entity reach distance with\nthe one set in Generic -> entityReachDistance"),
     TWEAK_ENTITY_TYPE_ATTACK_RESTRICTION("tweakEntityTypeAttackRestriction",false, "",    "Restricts which entities you are able to attack (manually).\nSee the corresponding 'entityAttackRestriction*' configs in the Lists category."),
@@ -87,8 +88,6 @@ public enum FeatureToggle implements IHotkeyTogglable, IConfigNotifiable<IConfig
     TWEAK_RENDER_LIMIT_ENTITIES     ("tweakRenderLimitEntities",            false, "",    "Enables limiting the number of certain types of entities\nto render per frame. Currently XP Orbs and Item entities\nare supported, see Generic configs for the limits."),
     TWEAK_REPAIR_MODE               ("tweakRepairMode",                     false, "",    "If enabled, then fully repaired items held in hand will\nbe swapped to damaged items that have Mending on them."),
     TWEAK_SCULK_PULSE_LENGTH        ("tweakSculkPulseLength",               false, true, "",    "Allows modifying the Sculk Sensor pulse length. Set the pulse length in Generic -> sculkSensorPulseLength"),
-    // fixme: is this still necessary? we can detect if the player have permission so it wont send too many requests
-    TWEAK_SERVER_ENTITY_DATA_SYNCER ("tweakServerEntityDataSyncer",         true,  "",    "Allows Tweakeroo to attempt to use the Server Data Syncer\nto obtain various Entity Data, such as for Shulker Boxes\n\n§6NOTE: This feature requires OP privileges to work, Tweakeroo will try to detect if you have enough permission."),
     TWEAK_SHULKERBOX_DISPLAY        ("tweakShulkerBoxDisplay",              false, "",    "Enables the Shulker Box contents display when hovering\nover them in an inventory and holding shift"),
     TWEAK_SIGN_COPY                 ("tweakSignCopy",                       false, "",    "When enabled, placed signs will use the text from\nthe previously placed sign.\nCan be combined with tweakNoSignGui to quickly place copies\nof a sign, by enabling that tweak after making the first sign."),
     TWEAK_SNAP_AIM                  ("tweakSnapAim",                        false, "",    KeybindSettings.INGAME_BOTH, "Enabled a snap aim tweak, to make the player face to pre-set exact yaw rotations"),

src/main/java/fi/dy/masa/tweakeroo/data/ServerDataSyncer.java

@@ -2,6 +2,7 @@
 
 import com.mojang.datafixers.util.Either;
 import fi.dy.masa.tweakeroo.Tweakeroo;
+import fi.dy.masa.tweakeroo.config.FeatureToggle;
 import fi.dy.masa.tweakeroo.mixin.IMixinDataQueryHandler;
 import net.minecraft.block.BlockEntityProvider;
 import net.minecraft.block.BlockState;
@@ -13,6 +14,7 @@
 import net.minecraft.client.network.DataQueryHandler;
 import net.minecraft.client.world.ClientWorld;
 import net.minecraft.entity.Entity;
+import net.minecraft.entity.EntityType;
 import net.minecraft.inventory.DoubleInventory;
 import net.minecraft.inventory.Inventory;
 import net.minecraft.nbt.NbtCompound;
@@ -52,11 +54,14 @@ public static void resetInstance()
      */
     private final Map<BlockPos, Pair<BlockEntity, Long>> blockCache = new HashMap<>();
     private final Map<Integer, Pair<Entity, Long>> entityCache = new HashMap<>();
-    private final Map<Either<BlockPos, Integer>, CompletableFuture<@Nullable NbtCompound>> pendingQueries = new HashMap<>();
-    private final Map<Integer, CompletableFuture<@Nullable NbtCompound>> pendingQueryFutures = new HashMap<>();
+    /**
+     * this map is not important, you can clear it anytime you want, but a bit more packets will be sent to the server
+     */
+    private final Map<Either<BlockPos, Integer>, CompletableFuture<@Nullable NbtCompound>> pendingQueriesByObject = new HashMap<>();
+    private final Map<Integer, CompletableFuture<@Nullable NbtCompound>> pendingQueriesById = new HashMap<>();
     private final ClientWorld clientWorld;
     /**
-     * if the client can query the server for block/entity data? null if not yet known.
+     * if the client can query the server for block/entity data? empty if not yet known.
      */
     private Optional<Boolean> yesIAmOp = Optional.empty();
 
@@ -104,27 +109,36 @@ public ServerDataSyncer(ClientWorld world)
     public void handleQueryResponse(int transactionId, NbtCompound nbt)
     {
         Tweakeroo.logger.debug("handleQueryResponse: id [{}] // nbt {}", transactionId, nbt);
-        pendingQueryFutures.remove(transactionId).complete(nbt);
 
-        if (nbt == null) return;
-        if (pendingQueryFutures.containsKey(transactionId))
+        CompletableFuture<@Nullable NbtCompound> future = pendingQueriesById.remove(transactionId);
+        if (future != null)
         {
+            future.complete(nbt);
             yesIAmOp = Optional.of(true);
         }
 
         if (blockCache.size() > 30)
         {
             blockCache.entrySet().removeIf(entry -> System.currentTimeMillis() - entry.getValue().getRight() > 1000);
+            pendingQueriesByObject.clear();
         }
         if (entityCache.size() > 30)
         {
             entityCache.entrySet().removeIf(entry -> System.currentTimeMillis() - entry.getValue().getRight() > 1000);
+            pendingQueriesByObject.clear();
         }
     }
 
     public Inventory getBlockInventory(World world, BlockPos pos)
     {
-        if (yesIAmOp.isPresent() && !yesIAmOp.get()) return null;
+        if (FeatureToggle.TWEAK_DISABLE_SERVER_DATA_SYNC.getBooleanValue())
+        {
+            return null;
+        }
+        else if (yesIAmOp.isPresent() && !yesIAmOp.get())
+        {
+            return null;
+        }
         Tweakeroo.logger.debug("getBlockInventory: pos [{}], op status: {}", pos.toShortString(), yesIAmOp);
 
         if (!world.isChunkLoaded(pos)) return null;
@@ -198,18 +212,19 @@ public CompletableFuture<NbtCompound> syncBlockEntity(World world, BlockPos pos)
         Either<BlockPos, Integer> posEither = Either.left(pos);
         if (MinecraftClient.getInstance().getNetworkHandler() != null)
         {
-            if (pendingQueries.containsKey(posEither))
+            if (pendingQueriesByObject.containsKey(posEither))
             {
-                return pendingQueries.get(posEither);
+                return pendingQueriesByObject.get(posEither);
             }
 
             CompletableFuture<NbtCompound> future = new CompletableFuture<>();
             DataQueryHandler handler = MinecraftClient.getInstance().getNetworkHandler().getDataQueryHandler();
             handler.queryBlockNbt(pos, it -> {
             });
-            pendingQueries.put(posEither, future);
-            pendingQueryFutures.put(((IMixinDataQueryHandler) handler).currentTransactionId(), future);
+            pendingQueriesByObject.put(posEither, future);
+            pendingQueriesById.put(((IMixinDataQueryHandler) handler).currentTransactionId(), future);
             future.thenAccept(nbt -> {
+                pendingQueriesByObject.remove(posEither);
                 if (!clientWorld.isChunkLoaded(pos) || nbt == null) return;
                 BlockState state = clientWorld.getBlockState(pos);
 
@@ -243,18 +258,19 @@ public CompletableFuture<NbtCompound> syncEntity(int networkId)
         Either<BlockPos, Integer> idEither = Either.right(networkId);
         if (MinecraftClient.getInstance().getNetworkHandler() != null)
         {
-            if (pendingQueries.containsKey(idEither))
+            if (pendingQueriesByObject.containsKey(idEither))
             {
-                return pendingQueries.get(idEither);
+                return pendingQueriesByObject.get(idEither);
             }
 
             CompletableFuture<NbtCompound> future = new CompletableFuture<>();
             DataQueryHandler handler = MinecraftClient.getInstance().getNetworkHandler().getDataQueryHandler();
             handler.queryEntityNbt(networkId, it -> {
             });
-            pendingQueries.put(idEither, future);
-            pendingQueryFutures.put(((IMixinDataQueryHandler) handler).currentTransactionId(), future);
+            pendingQueriesByObject.put(idEither, future);
+            pendingQueriesById.put(((IMixinDataQueryHandler) handler).currentTransactionId(), future);
             future.thenAccept(nbt -> {
+                pendingQueriesByObject.remove(idEither);
                 if (nbt == null) return;
                 if (clientWorld.getEntityById(networkId) != null)
                 {
@@ -280,6 +296,16 @@ public CompletableFuture<NbtCompound> syncEntity(int networkId)
 
     public @Nullable Entity getServerEntity(Entity entity)
     {
+        if (FeatureToggle.TWEAK_DISABLE_SERVER_DATA_SYNC.getBooleanValue())
+        {
+            return null;
+        }
+        else if (yesIAmOp.isPresent() && !yesIAmOp.get())
+        {
+            return null;
+        }
+        Tweakeroo.logger.debug("getServerEntity: id [{}], type {}, op status: {}", entity.getId(), EntityType.getId(entity.getType()), yesIAmOp);
+
         Entity serverEntity = getCache(entity.getId());
         if (serverEntity == null)
         {
@@ -292,5 +318,7 @@ public CompletableFuture<NbtCompound> syncEntity(int networkId)
     public void recheckOpStatus()
     {
         yesIAmOp = Optional.empty();
+        pendingQueriesByObject.clear();
+        pendingQueriesById.clear();
     }
 }

src/main/java/fi/dy/masa/tweakeroo/mixin/MixinClientPlayNetworkHandler.java

@@ -46,7 +46,7 @@ private void onPlayerDeath(DeathMessageS2CPacket packetIn, CallbackInfo ci)
     )
     private void onCommandTree(CallbackInfo ci)
     {
-        if (FeatureToggle.TWEAK_SERVER_ENTITY_DATA_SYNCER.getBooleanValue())
+        if (!FeatureToggle.TWEAK_DISABLE_SERVER_DATA_SYNC.getBooleanValue())
         {
             // when the player becomes OP, the server sends the command tree to the client
             ServerDataSyncer.getInstance().recheckOpStatus();

src/main/java/fi/dy/masa/tweakeroo/mixin/MixinDataQueryHandler.java

@@ -24,9 +24,9 @@ public class MixinDataQueryHandler
     )
     private void queryResponse(int transactionId, NbtCompound nbt, CallbackInfoReturnable<Boolean> cir)
     {
-        Tweakeroo.logger.warn("MixinDataQueryHandler: nbt {}", nbt.toString());
+        Tweakeroo.logger.debug("MixinDataQueryHandler: nbt {}", nbt.toString());
 
-        if (FeatureToggle.TWEAK_SERVER_ENTITY_DATA_SYNCER.getBooleanValue())
+        if (!FeatureToggle.TWEAK_DISABLE_SERVER_DATA_SYNC.getBooleanValue())
         {
             ServerDataSyncer.getInstance().handleQueryResponse(transactionId, nbt);
         }

src/main/java/fi/dy/masa/tweakeroo/renderer/RenderUtils.java

@@ -148,11 +148,9 @@ public static void renderInventoryOverlay(MinecraftClient mc, DrawContext drawCo
                 shulkerBoxBlock = (ShulkerBoxBlock) blockTmp;
             }
 
-            if (world instanceof ServerWorld realWorld)
-            {
-                inv = fi.dy.masa.malilib.util.InventoryUtils.getInventory(realWorld, pos);
-            }
-            else if (FeatureToggle.TWEAK_SERVER_ENTITY_DATA_SYNCER.getBooleanValue() && world.getBlockState(pos).getBlock() instanceof BlockEntityProvider)
+            inv = fi.dy.masa.malilib.util.InventoryUtils.getInventory(world, pos);
+            if (world.isClient && world.getBlockState(pos).getBlock() instanceof BlockEntityProvider
+                    && !FeatureToggle.TWEAK_DISABLE_SERVER_DATA_SYNC.getBooleanValue())
             {
                 inv = ServerDataSyncer.getInstance().getBlockInventory(world, pos);
             }
@@ -162,7 +160,7 @@ else if (trace.getType() == HitResult.Type.ENTITY)
             Entity entity = ((EntityHitResult) trace).getEntity();
 
             if (entity.getWorld().isClient &&
-                FeatureToggle.TWEAK_SERVER_ENTITY_DATA_SYNCER.getBooleanValue())
+                !FeatureToggle.TWEAK_DISABLE_SERVER_DATA_SYNC.getBooleanValue())
             {
                 Entity serverEntity = ServerDataSyncer.getInstance().getServerEntity(entity);
                 if (serverEntity != null)