Jump to content

One client side TileEntity getting replaced with a new instance for some reason


BenignBanana

Recommended Posts

Hey guys,

After hours of banging my head against a wall I finally managed to narrow down my code not working to one specific issue. For some unknown reason, one of my multiblock TileEntities is getting replaced by a new instance after a sync from the server. I have no idea what's causing this, the other identical TE is working perfectly fine, I'm at the end of my wisdom and I just hope that maybe someone here has any idea. The code is way too all over the place to post here (though you're welcome to check out the GitHub for the main library  and the mod itself, most of the code in question is in the library and all relevant classes have multiblock or sync in them). I did take some fairly detailed debug notes though, so here they are: 

/* type | side | type mem location | syncable id mem location | action */
boiler server 10066 10053 setting proper id
boiler server 10096 10143 setting proper id
tank server 10105 10112 setting proper id
tank server 10100 10037 setting proper id

tank client 10182 10154 reading proper id from stream
boiler client 10209 10196 reading proper id from stream
boiler client 10174 10222 reading proper id from stream
tank client 10259 10246 reading proper id from stream

/* type | side | coords | debug uuid msb | debug uuid lsb | actions performed */
boiler server 399 4 2100 7406611159510238969 -6004429813246348525 set value in rebuild | getActualBlockState()
boiler server 400 4 2100 8627676692757629106 -9168228343733114012 set value in rebuild
tank server 399 5 2100 984907512996252097 -8261117959805136242 set value in rebuild // triggered rebuild
tank server 400 5 2100 5216046578638867465 -9161033129145849707 set value in rebuild

boiler client 399 4 2100 -777796711269906213 -6909084835805274391 called 2x in getActualBlockState() | read from stream
boiler client 400 4 2100 1875314197142261539 -4929778570773350453 called 2x in getActualBlockState() | read from stream
tank client 400 5 2100 -2113191912659205197 -4780008886862939808 read from stream
tank client 399 5 2100 -6765782617268665069 -6187872588524286999 read from stream

/* like above, but the UUID combined with the BlockPos is the important thing to look at.
boiler client 399 4 2100 -1982676738952378984 -7018763097901900451 getActualBlockState() new TE!!!!
boiler client 400 4 2100 1875314197142261539 -4929778570773350453 getActualBlockState() this one is the same

 

Keep in mind that each of the two sections were taken in a single update tick after completing the multiblock.

 

I seriously don't know what to do anymore. Hopefully someone here can figure it out.

Edited by BenignBanana
flipped debug notes to be more in execution order
Link to comment
Share on other sites

I don't fully understand your code, but could it be that the IBlockState at that position is changing, triggering a refresh of the TileEntity (as determined by FruityTileEntity#shouldRefresh)?

Please don't PM me to ask for help. Asking your question in a public thread preserves it for people who are having the same problem in the future.

Link to comment
Share on other sites

9 hours ago, Choonster said:

I don't fully understand your code, but could it be that the IBlockState at that position is changing, triggering a refresh of the TileEntity (as determined by FruityTileEntity#shouldRefresh)?

I don't see what would set a new BlockState though, especially considering only one of the two TileEntityBoilers has this issue. I also don't override shouldRefresh and in the default TileEntity that just returns true if the Block changes (which it obviously doesn't). After doing a bunch more debugging with a clearer mind I noticed some other things. For one, I made a new world to exclude the possibility of a world corruption during development. This didn't help. I also added a third block to the structure and tracked the TileEntities over quite a few multiblock breaks/rebuilds. What I found is that it's not one specific TileEntity that gets refreshed, but they almost always refresh one at a time. There were two syncs where 2 changed at once, but on the vast majority it was just one, going highest X to lowest X in order.

 

I also let the BlockState refresh from just normal render refreshes and the TileEntities never changed, combined with the fact that this only happens on the client side it means the bug must be somewhere in the sync code. I pretty much straight ported that from OpenModsLib which was written for 1.7 (aka pre BlockStates), so there might be a sneaky bug in there I can't find. I hardly changed anything in the sync system, so it might be easier for you to understand the Java equivalent code here: https://github.com/OpenMods/OpenModsLib/tree/master/src/main/java/openmods/sync

 

I've been looking at this code for way too long and I might just be missing something really obvious. I'll keep looking but maybe you can figure something out.

Link to comment
Share on other sites

FruityTileEntity overrides TileEntity#shouldRefresh to return true when the IBlockState has changed, not when the Block has changed.

 

If you put a breakpoint in BlockBoiler#createNewTileEntity with the condition worldIn.isRemote (i.e. the method is being called on the logical client), can you see what's causing the new TileEntity to be created?

 

Side note: Don't use ITileEntityProvider, override Block#hasTileEntity(IBlockState) and Block#createTileEntity instead.

Please don't PM me to ask for help. Asking your question in a public thread preserves it for people who are having the same problem in the future.

Link to comment
Share on other sites

Hmm, so the original issue may or may not still be there, but after removing that shouldRefresh override and defaulting it to the vanilla behaviour, the server side no longer gets the onBlockPlacedBy event, and onBlockAdded() doesn't seem to fire at all, period. My guess is that shouldRefresh has something to do with sending a block update to the server, but I can't find the relevant code. Any ideas?

Link to comment
Share on other sites

Post your updated code.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable.  If you think this is the case, JUST REPORT ME.  Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

 

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

 

DO NOT PM ME WITH PROBLEMS. No help will be given.

Link to comment
Share on other sites

TileEntity#shouldRefresh has nothing to with whether Block#onBlockPlaced or Block#onBlockAdded are called, so I'm not sure why that's happening.

 

The default behaviour of TileEntity#shouldRefresh for non-vanilla TileEntities is actually the same as what you had in FruityTileEntity(return true when the IBlockState changes), so removing the override shouldn't have changed anything.

Please don't PM me to ask for help. Asking your question in a public thread preserves it for people who are having the same problem in the future.

Link to comment
Share on other sites

 

41 minutes ago, Draco18s said:

Post your updated code.

I always forget to commit when I'm trying to debug stuff. It's updated now.

36 minutes ago, Choonster said:

TileEntity#shouldRefresh has nothing to with whether Block#onBlockPlaced or Block#onBlockAdded are called, so I'm not sure why that's happening.

 

The default behaviour of TileEntity#shouldRefresh for non-vanilla TileEntities is actually the same as what you had in FruityTileEntity(return true when the IBlockState changes), so removing the override shouldn't have changed anything.

It seems that after restarting the client the events fire once on the server side (including blockAdded), but after that it only fires client side and blockAdded stops firing altogether. I honestly don't have a clue what's going on. I don't just have a breakpoint in the events themselves, I have breakpoints on lots of spots down the chain and the only ones that are getting hit are onBlockPlacedBy and teh following world.isRemote check in BlockMultiblockPart. I can't figure out what's happening. I made sure nothing down the inheritance chain is overriding those functions either.

 

Here's the listing of the relevant functions with break points marked as a comment

    override fun onBlockPlacedBy(worldIn: World, pos: BlockPos, state: IBlockState, placer: EntityLivingBase, stack: ItemStack) {
        worldIn.setBlockState(pos, state.withProperty(FACING, placer.horizontalFacing.opposite), 2) //BP: breaks once with worldIn instanceof WorldClient
        placeEvent.fire(BlockPlacedEvent(worldIn, pos, state, placer, stack))
        super.onBlockPlacedBy(worldIn, pos, state, placer, stack)
    }

    override fun onBlockAdded(worldIn: World, pos: BlockPos, state: IBlockState) {
        addEvent.fire(BlockAddedEvent(worldIn, pos, state)) //BP: never triggers
        super.onBlockAdded(worldIn, pos, state)
    }

    init {
        placeEvent += {
            if(!it.world.isRemote) { //BP: Triggers once with it.world instanceof WorldClient
                val te = it.world.getTileEntity(it.pos) as TileEntityMultiblockPart
                te.rebuild(it.pos)
            }
        }
        breakEvent += {
            if(!it.world.isRemote) {
                val te = it.world.getTileEntity(it.pos) as TileEntityMultiblockPart
                if(te.multiblockId.value != -1) (it.world.getEntityByID(te.multiblockId.value) as? EntityMultiblock)?.destroy()
            }
        }
    }

This is the hardest to track down bug I've ever had. Seriously.

Link to comment
Share on other sites

Probably not because no one else uses Krolin...sorry, Kotlin to program mods.  The fact that onBlockAdded is never called is a complete mystery to us.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable.  If you think this is the case, JUST REPORT ME.  Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

 

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

 

DO NOT PM ME WITH PROBLEMS. No help will be given.

Link to comment
Share on other sites

2 hours ago, diesieben07 said:

It would be best if you could provide use with a working Git repository of your mod.

I posted both in the OP. It's completely up to date after draco reminded me to commit.

 

2 hours ago, Draco18s said:

Probably not because no one else uses Krolin...sorry, Kotlin to program mods.  The fact that onBlockAdded is never called is a complete mystery to us.

Well yeah. I know that Kotlin hasn't really caught on yet outside the Android community, but most of the code is similar enough to Java to be understandable. Either way most of the more complicated kotlin code doesn't really deal with the problem, it's just that very simple bit of code I posted above.

 

Also, I mostly asked "No one?" as a way to push the thread back up, since the conversation kind of stopped after yesterday afternoon and the thread was about to leave the first page when I checked back today.

 

Sorry if I'm being a bit pushy, this is just a really big issue that prevents me from further working on my mod and despite my 8 years Java experience I cannot for the life of me figure this out by myself. I hate asking others to help me debug my own code, I only posted here because I'm pretty desperate.

Link to comment
Share on other sites

If your TE is being replaced when you don't want it replaced, then the logic in your shouldRefresh method is probably faulty (or not being reached, resulting in default behavior, which replaces the TE for every state change).

 

Make sure you have overridden shouldRefresh, and make sure that its logic does replace the TE when it needs to be cleaned up (e.g. when the block class changes) but not when you want to keep your TE during a state change. Step through it in the debugger to watch it work.

The debugger is a powerful and necessary tool in any IDE, so learn how to use it. You'll be able to tell us more and get better help here if you investigate your runtime problems in the debugger before posting.

Link to comment
Share on other sites

1 hour ago, jeffryfisher said:

If your TE is being replaced when you don't want it replaced, then the logic in your shouldRefresh method is probably faulty (or not being reached, resulting in default behavior, which replaces the TE for every state change).

 

Make sure you have overridden shouldRefresh, and make sure that its logic does replace the TE when it needs to be cleaned up (e.g. when the block class changes) but not when you want to keep your TE during a state change. Step through it in the debugger to watch it work.

I totally misread that vanilla code, I thought if it ISN'T vanilla it checks if the block changed, otherwise if the state changed. Seems like its the other way around. Derp. Anyways, still have the problem of onBlockPlacedBy not being called server side. I'm moving my way through all the internal Minecraft/Forge code atm to see why this isn't working. Hopefully I can find something.


Edit: Is there some kind of Discord or something for Forge development help? I don't want to spam this forum with updates on the debugging.

Edited by BenignBanana
Link to comment
Share on other sites

Alright I finally figured out what was going on with the events not being called server side. Turns out in this new world, unlike before, the multiblock parts had different Y coordinates instead of different X coordinates, which caused an infinite loop in my rebuild function (due to a mistake of mine), but I couldn't catch this in step through because the logic was in a lambda delegate (which is executed in a separate context, hence no stepping). As a result the server got frozen and any further events were no longer sent. I thought the whole debugger hanging up was an issue because of spending too long in paused mode (which happens fairly frequently), but after a few restarts I noticed it always happened in the same spot, allowing me to spot the issue.


I learned my lesson: Always replace delegates with regular for loops while debugging. The issue with TEs being replaced is also fixed thanks to jeffryfisher's help.

 

The question about a discord or something still stands though, I'd much rather ask for help in a chat program if it's something that won't be useful to see for anyone else in the future.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Announcements



×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.