[1.19] Migrating a mod from 1.18

[Quipex]

Hello, first of all, I'm new to forge so if I'm missing some basic things, feel free to point me into it.

I'm trying to migrate a minecraft mod from 1.18 to 1.19. Its pretty much done, but there is one bug that I can't resolve.

On the original mod (1.18) there is packet handler that takes care of the main business logic: it scans cube of a configured radius, finds chests and fills them from player's inventory. Here's github link to the method that does the job.

On contrary, my migrated (1.19) method does the same. The main issue is that on the Line 184 after we map BlockPos to BlockEntity and filter by NotNull, we get  0 blocks (at 1.18 it determines everything fine):

var blockEntities = BlockPos.betweenClosedStream(minX, minY, minZ, maxX, maxY, maxZ)
                .map(world::getBlockEntity)
                .toList();
DropOff.LOGGER.debug("Found {} block entities", blockEntities.size());

var nonNulls = blockEntities.stream()
                .filter(Objects::nonNull).toList();
DropOff.LOGGER.debug("Found {} non-nulls", nonNulls.size());

This are the sources of Level#getBlockEntity(BlockPos):

   @Nullable
   public BlockEntity getBlockEntity(BlockPos p_46716_) {
      if (this.isOutsideBuildHeight(p_46716_)) {
         return null;
      } else {
         return !this.isClientSide && Thread.currentThread() != this.thread ? null : this.getChunkAt(p_46716_).getBlockEntity(p_46716_, LevelChunk.EntityCreationType.IMMEDIATE);
      }
   }

So I guess my issue lays in `Thread.currentThread() != this.thread`.

 

I reckon I did something wrong with the event listeners which led to this problem. Please help me troubleshoot the problem.

FYI: 1.19 repo, 1.18 repo

Edited October 24, 2022 by Quipex

[warjort]

You probably want to look at "handling packets" here: https://forge.gemwire.uk/wiki/SimpleChannel

i.e. making sure you run on the correct thread so you are doing things in a thread safe way and not running concurrently with other stuff on the networking thread(s)

[Quipex]

@warjort thanks for the reply, I've tried wrapping the whole before ctx.get().setPacketHandled(true); inside ctx.get().enqueueWork(lambda) but it acted the same. Am I missing something else?

[Quipex]

What surprises me is that the original 1.18 mod does not wrap inside ctx.get().enqueueWork(lambda) and works just fine

Edited October 24, 2022 by Quipex

[warjort]

The 1.18.1 Level.getBlockEntity() also has that thread check, so I don't see how your code works there?

I suggest you add some logging or use a debugger to figure out what is really happening (on both versions).

[warjort]

I've just been testing your 2 branches.

In 1.18.2 the thread is usually already the ServerThread so it works. But this isn't guaranteed, it could be a networking thread.

In 1.19.2 the thread is usually a networking thread so it fails.

 

When I wrap your handle() code inside a

ctx.get().enqueueWork(() -> {
// original code here
});

it is always on the server thread and so works.

 

So it looks like you didn't change your code properly, maybe you didn't save it?

NOTE: You will need to fix this for 1.18.2 as well. As I said above, this code could run on "any" thread unless you force it onto the ServerThread.

[Quipex]

UPD: saw your comment above, let me check that

 

Ok, so I've debugged and now I can confirm that on the 1.18 Thread.currentThread() != this.thread produces false, when on 1.19 it gives true. Only things that come to my mind are: I made some event handlers static (to compare: 1.18 and 1.19)

Edited October 24, 2022 by Quipex
too late to respond

[Quipex]

Good news, now `Thread.currentThread() != this.thread` indeed produces false, but `this.getChunkAt(p_46716_).getBlockEntity(p_46716_, LevelChunk.EntityCreationType.IMMEDIATE)` always gives null for some reason

[Quipex]

@warjort forgot to ping you above 👆

[Quipex]

12 minutes ago, warjort said:

NOTE: You will need to fix this for 1.18.2 as well. As I said above, this code could run on "any" thread unless you force it onto the ServerThread.

Weird thing: I've wrapped 1.18 code inside enqueueWork and now it also returns only nulls..

[warjort]

You will need to debug that for yourself.

Maybe the chunk you are referencing isn't loaded or you have "broken" the BlockEntity in your test world during some previous testing?

I've just been testing with a chest and got it to work.

Try creating a new test world so you know you don't have corrupt data.

 

BTW: "Pinging" does nothing except adding noise the thread.

If it really did something I would I turn it off. 🙂 

[warjort]

This is the change I made to get 1.19.2 to work:

diff --git a/src/main/java/tfar/quickstack/networking/C2SPacketRequestDropoff.java b/src/main/java/tfar/quickstack/networking/C2SPacketRequestDropoff.java
index 7a1d17a..ee58ebb 100644
--- a/src/main/java/tfar/quickstack/networking/C2SPacketRequestDropoff.java
+++ b/src/main/java/tfar/quickstack/networking/C2SPacketRequestDropoff.java
@@ -58,6 +58,7 @@ public class C2SPacketRequestDropoff {
     }

     public void handle(Supplier<NetworkEvent.Context> ctx) {
+ctx.get().enqueueWork(() -> {
         ServerPlayer player = ctx.get().getSender();
         Set<InventoryData> nearbyInventories = getNearbyInventories(player);

@@ -97,6 +98,7 @@ public class C2SPacketRequestDropoff {
                 new S2CReportPacket(itemsCounter, affectedContainers, nearbyInventories.size(),
                         rendererCubeTargets));
         ctx.get().setPacketHandled(true);
+});
     }

     public void dropOff(Player player, IItemHandler target, InventoryData data) {
@@ -172,6 +174,7 @@ public class C2SPacketRequestDropoff {
         Level world = player.level;
         DropOff.LOGGER.debug("World info: {}", world);
         DropOff.LOGGER.debug("Is client: {}", world.isClientSide());
+        DropOff.LOGGER.debug("" + Thread.currentThread());

         DropOff.LOGGER.debug("Scanning x({},{}) y({},{}), z({},{})", minX, maxX, minY, maxY, minZ, maxZ);
         var blockEntities = BlockPos.betweenClosedStream(minX, minY, minZ, maxX, maxY, maxZ)

 

[Quipex]

5 minutes ago, warjort said:

Maybe the chunk you are referencing isn't loaded or you have "broken" the BlockEntity in your test world during some previous testing?

No, just tested on a brand new world, it must be something else.

6 minutes ago, warjort said:

I've just been testing with a chest and got it to work.

Just to make sure I got you right: it highlights the chests and prompts in console how many chests were checked in total (not 0)?

[warjort]

Quote

[19:50:14] [Netty Local Client IO #0/INFO] [minecraft/ChatComponent]: [System] [CHAT] [DropOff]: 1 items moved to 1 containers of 1 checked in total.

And I got the highlighted chest.

I am just intuiting what it supposed to do. You haven't explained it.

 

But anyway, it's not really my task to debug your code. I only did the stuff above because what you were talking about didn't make sense and I wanted to know why.

 

[Quipex]

After some time it and refactoring it magically began to work. Seems like some weird caching shit related to 'run' folder. Thanks for the help!