Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Ancient Blocks being Invalid Stabilisers #42

Merged

Conversation

54M44R
Copy link

@54M44R 54M44R commented Nov 4, 2024

TBBLock & TBBLockDeco are identical. All instances of TBBLock & TBSidedBlock are valid stabilisers
Adds the proper tooltip to blocks which are valid stabilisers
@chochem chochem added the ongoing freeze - do not merge Not just a bug fix and thus affected by a current freeze for a upcoming version label Nov 4, 2024
@Dream-Master Dream-Master requested a review from a team November 4, 2024 22:58
@Dream-Master Dream-Master merged commit 0caf835 into GTNewHorizons:master Nov 5, 2024
1 check passed
@Dream-Master
Copy link
Member

It’s a fix not need to freeze

@chochem
Copy link
Member

chochem commented Nov 5, 2024

its not a fix. Its changing a cosmetic block to be a stabilizer. More importantly though: it is 2 PRs, you cant only merge one.

@Dream-Master
Copy link
Member

Ok let me check

@Dream-Master
Copy link
Member

i send the dev a pm to fix the other pr and we are good i hope

@54M44R
Copy link
Author

54M44R commented Nov 5, 2024

its not a fix. Its changing a cosmetic block to be a stabilizer. More importantly though: it is 2 PRs, you cant only merge one.

The original Thaumic Bases code does show they were not intended to be stabilizers, though common knowledge is that they are due to wikis stating they should be (along with the tooltip issue)

This was done more so for existing consistency as in commit d74f599 from /pull/17 has introduced additional stabilizers due to the method canStabaliseInfusion() being hardcoded to return true. The two options would be to either:

  • Revert these changes which would cause players using stabilizers such as Ancient Cobblestone to be invalid
  • Allow all decorative blocks to be used as stabilizers

I chose the latter due to Ancient Cobblestone being very cheap already. Since the alternatives are far more expensive, it would only benefit players who preferred the visuals of the ancient blocks

Whether this choice was objectively correct or not I'm not sure. I am more than happy to write another PR to revert these changes along with /pull/17. What are your thoughts @Dream-Master @chochem ?

@Alastors
Copy link

Alastors commented Nov 5, 2024

Respectfully, infusion stabilizers are effectively free, partly because of GTNH's cross mod compat.

Another example besides ancient cobble would be salis mundis blocks, salis mundis of course can be duplicated and turned into blocks for free through the use of a rosa mysteria farm

@chochem
Copy link
Member

chochem commented Nov 5, 2024

I don't particularly care either way as long as they are not cheaper than ancient cobblestone. (e.g. no chisel please)

And on the history: ancient Cobblestone was not a stabilizer for most of the 10 year history of GTNH either. After it was made a stabilizer (I think thats the PR you linked), we then adjusted the recipe slightly. Actually that was me I think. And yes, the current very cheap ancient cobble recipe is the 'nerfed' version. :P

@chochem
Copy link
Member

chochem commented Nov 5, 2024

dream just wants you to resolve the review in the other PR so that he can merge that as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ongoing freeze - do not merge Not just a bug fix and thus affected by a current freeze for a upcoming version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple Thaumic Bases blocks incorrectly labeled as infusion stabilizers
5 participants