Jump to content

Question on Forge's mod to getTileEntities() in WorldServer.java


mikeprimm

Recommended Posts

This is specific to https://github.com/MinecraftForge/MinecraftForge/blob/master/patches/minecraft/net/minecraft/world/WorldServer.java.patch#L171.

 

Over on Spigot, we developed our own version of this performance patch before realizing that the Forge team had already discovered and addressed it.  One debate we're still having is on the need for the 'isInvalid()' check that is being made in the Forge version - I've been on the page that you guys have had it for a while, so I'm inclined to trust that you had a good reason, but one of my counterpart is arguing that it doesn't appear to be necessary.  If anyone involved with this change can recall, was there a particular need or issue addressed by checking for the isInvalid()?  The cases my associate has identified appear to avoid the issue of the invalid TEs being on the chunk's list by the time the call is used, but I'm not confident that all the cases have been considered.

 

If you're interested, the discussion is on this PR - https://github.com/EcoCityCraft/Spigot/pull/21#discussion_r3192913.  Thanks for any insight that can be offered.

Link to comment
Share on other sites

Due to several changes where we defer removal until later times, the tile entities in the chunk can be invalid, so yes, this is needed.

Super - thanks for the info.  So, just to be 100% clear, to your knowledge, it was needed in Forge due to other Forge-related changes, versus being needed to preserved "vanilla" behavior, correct?

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.