Refined Storage conduit GUI generating large number of chunk updates/redraws.
WillWill56 opened this issue ยท 8 comments
Issue Description:
While accessing the GUI of a Refined Storage conduit (e.g. to add an import or export filter), a large number of chunk updates & redraws are triggered for the chunk that it's placed in. The rate seems to be tied to the tick rate of the server.
In a chunk with a high redraw cost (e.g. lots of conduits, machines, even just a lot of blocks with connected textures), this can destroy the game's frame rate, when otherwise the frame rate is fine in that chunk.
Conduit GUIs for other types of conduits do not appear to have this bug, unless they are placed in the same block space as a Refined Storage conduit.
What happens:
Chunk updates/redraws are constantly triggered while using the RS conduit GUI.
What you expected to happen:
Chunk updates/redraws are only triggered when necessary, e.g. disabling a Refined Storage conduit connection requires an update to the visual appearance of the conduit block.
Steps to reproduce:
(Tested in a new world, in a custom modpack containing only JEI, Ender IO, Refined Storage and Chisel)
Basic reproduction:
- Place down a machine, e.g. SAG mill
- Place a Refined Storage conduit connected to the machine
- Press F3 to display debug statistics (frame rate and chunk updates)
- Observe that the chunk update counter stays at 0 most of the time.
- Right click the Refined Storage conduit where it connects to the machine to open the conduit GUI
- Observe that the chunk update counter constantly shows a value of about 20 (sometimes 40 or other multiples of 20) while the conduit GUI is open. Frame rate will probably remain at a good value.
Reproduction of poor frame rate:
- Using F3+G to enable visible chunk borders, pick a fairly empty, ordinary chunk, e.g. in a plains biome or desert biome.
- Perform the "Basic reproduction" steps to show that frame rate remains good/high in the chunk you chose.
- Place down a decent number of "redraw-intensive" blocks in the chunk. Any block with connected textures from Chisel will work fine, from something as simple as "Chiseled Stone - Dent", to something crazy like "Futura Block - Fabulously Wavy". How many blocks you need to place down I assume depends on how powerful your PC is, but roughly 4 stacks of Chisel blocks was more than enough for my testing. (It's worth noting that Chisel is not the only mod with redraw-intensive blocks, nor is it the worst offender.)
- Perform the "Basic reproduction" steps again in the chunk, and observe a (potentially large) drop in frame rate.
Affected Versions (Do not use "latest"):
- EnderIO: 5.0.38 (also 5.0.36)
- EnderCore: 0.5.43 (also 0.5.41)
- Minecraft: 1.12.2
- Forge: 14.23.5.2784
- SpongeForge? no
- Optifine? no
- Single Player and/or Server? Tested and occurs in both situations, but of course only the client gets frame rate problems.
Your most recent log file where the issue was present:
No crash occurred, and likely no relevant entries in log file, this is a performance-related bug.
TL;DR
Imagine a redstone conduit switching on/off 20 times per second. That's a decent analogy for what the Refined Storage conduit is doing when its conduit GUI is open.
@EpicSquid are you doing anything different in that one gui?
Then it's weird. I'll try to reproduce it tonight.
(Technical: TE data is synced to the client 20 times a second while a GUI is open to keep the GUI updated, but the world should not re-render for that unless there's a change that affects rendering. I suspect(ed) the conduit bundle may be too 'dumb' to notice that difference and re-render always, but then this would happen for all conduit GUIs, not just RS.)
Nope, nothing different there. It has less going on than an item or fluid conduit.
I used Sampler's packet tracker to capture some TE update packets for RS conduits and item conduits separately, but I can't see anything obvious. Both have "forceClientRerender" set to the same value in their packets. Anything else I should be looking for/would you like me to post the capture files?
Yes, that hashcode is for the visuals, it is also used to cache the dynamically created conduit models.
I think the RS conduit doesn't have things like colored channel bands and in/out arrows, so the base method would really be enough. I'll add this tomorrow unless epic does it earlier.
I think I might have found the problem. RefinedStorageConduit doesn't implement IConduitComponent, so TileConduitBundle.makeConduitHashCode() resorts to using the default hashcode (which is just Object.hashCode() and different for every object). TileConduitBundle.onAfterNbtRead() notices the change in hash code and sets clientUpdated to true, meaning every tile entity update triggers a redraw.
that sounds plausible. and it's the kind of error we'd never spot... thank you very much!
No problem. Set up a dev environment just so I could set breakpoints in the code and figure out what was going on, I was curious to see what was causing it.
I'm guessing only things that affect the visual appearance of the conduit should change the hash code? If so, the fix is potentially less than one line, since AbstractConduit already has an implementation for IConduitComponent, all that's needed is to add "implements IConduitComponent" to the class declaration for RefinedStorageConduit and it will use the implementation in the base class. I'm testing it right now and it appears to have solved the problem without breaking anything else...