Skip to content

Fixed furnace BurnTime overflow with high BurnTime fuels - #990

Open
playfuldoggo wants to merge 5 commits into
GTNewHorizons:masterfrom
playfuldoggo:master
Open

Fixed furnace BurnTime overflow with high BurnTime fuels#990
playfuldoggo wants to merge 5 commits into
GTNewHorizons:masterfrom
playfuldoggo:master

Conversation

@playfuldoggo

Copy link
Copy Markdown

Summary

Fixes this bug for vanilla furnaces GTNewHorizons/Et-Futurum-Requiem#113
(original issue)

Changes the "BurnTime" component in the Furnace tile entity to use an Integer tag instead of a Short tag to prevent BurnTime overflow.

The write uses a Redirect injector, but the read is more complicated because I can't cleanly change the return signature of getShort. It injects itself at the return of the readNBT, allowing it to overwrite the short-casted version of BurnTime. It also has a check for if the Integer version of the BurnTime tag has been made, as to not overwrite the BurnTime with 0 when updating the pack.

I can use more direct ways to change the readNBT part to use an Integer, but this seemed like the best option to me?
alexdoru please tell me how i'm wrong :)

Todo

Fix this bug in all other BurnTime implementations, I know that at least Natura nether furnaces still use shorts too. (outside of this repo's scope)

Checklist

  • I have tested this PR in DevEnv
  • I have tested this PR in Fullpack
  • This PR is in compliance with the GTNH AI Policy
  • This PR requires another PR in order to merge

Comment on lines +42 to +43
private void hodgepodge$readBurnTime(TileEntityFurnace instance, int value,
@Local(argsOnly = true) NBTTagCompound compound) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need this @Local(argsOnly = true) annotation. @Redirect will use the method's arguments for any extra parameters you declare on the injected method, so just adding a plain old NBTTagCompound compound parameter would link it to the first parameter of the containing method.

Suggested change
private void hodgepodge$readBurnTime(TileEntityFurnace instance, int value,
@Local(argsOnly = true) NBTTagCompound compound) {
private void hodgepodge$readBurnTime(TileEntityFurnace instance, int value, NBTTagCompound compound) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants