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

Implement new chests (from @unicornbloods) #16

Merged
merged 24 commits into from
Mar 3, 2024
Merged

Conversation

Caedis
Copy link
Member

@Caedis Caedis commented Mar 3, 2024

See PR: #14

Confirmed working in pack.

Need someone to test in an existing world that already has iron chests

@Caedis Caedis marked this pull request as draft March 3, 2024 03:06
@Caedis Caedis marked this pull request as ready for review March 3, 2024 03:58
@Caedis Caedis requested a review from a team March 3, 2024 03:58
Copy link

@Pilzinsel64 Pilzinsel64 left a comment

Choose a reason for hiding this comment

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

Ist that intended to use ingotNetherit for dark steel chest? Not sure if they have the same oredict entry.

@Pilzinsel64
Copy link

Pilzinsel64 commented Mar 3, 2024

Also, would it be an idea to also add a config to control netherit/dark steel variants instead of the dreamcraft mod, same as steel/silver variants?
For example, we have both in my private modpack and I either would like to amenable both or only enable dark steel.

I am going to test this PR later this day on a modpack outside gtnh (on my test server) with many gtnh mods and someothers. But maybe someone's else still want to try this on an existing gtnh modpack wold.

@Pilzinsel64 Pilzinsel64 requested a review from a team March 3, 2024 07:48
@Caedis
Copy link
Member Author

Caedis commented Mar 3, 2024

Feel free to make the changes, just remember that this mod is on the passive list

@Pilzinsel64
Copy link

Pilzinsel64 commented Mar 3, 2024

Alright, I tested this with some scenarious by turning the config enableDarkSteelChests on and off. Existing chests keep like they are but the upgrades mappes fine. There are no missing mappens for FML, so the server starts fine without problem.

What I changed in the source code:

  • Add a config enableDarkSteelChests wich controls if dark steel chest or nehterit chests will be choosen.
  • Use the presence of dreamcraft mod as default value for enableDarkSteelChests.
  • Add mappins for switching between dark steel and netherite chests.
  • Add translation for netherite upgrade.
  • Use ingotDarkSteel for dark steel chests (fixes the non-GTNH recipe)

I've seen that the mappings of turning off enableSteelChest are still missing. I consider submitting a seperated PR for this later as it has nothing todo with this PR.

@Pilzinsel64
Copy link

just remember that this mod is on the passive list

Sure. That's why I asked before. 😊

@Pilzinsel64 Pilzinsel64 requested review from a team and removed request for a team March 3, 2024 09:12
@Pilzinsel64
Copy link

Pilzinsel64 commented Mar 3, 2024

I request another dev to review since I added own changes and I am not longer in the position to review this. However, from my side it's a OK. See my comment above.

@Dream-Master
Copy link
Member

works and tested in GTNH full pack

@Dream-Master Dream-Master merged commit 40aca9a into master Mar 3, 2024
1 check passed
@Dream-Master Dream-Master deleted the newChests branch March 3, 2024 11:24
@Pilad
Copy link

Pilad commented Mar 12, 2024

Silver chest not working
изображение

@Pilad
Copy link

Pilad commented Mar 14, 2024

See PR: #14

Confirmed working in pack.

Need someone to test in an existing world that already has iron chests

Check please silver chest.

@Caedis
Copy link
Member Author

Caedis commented Mar 14, 2024

@Pilad Can you confirm that with the latest nightly?

@Pilad
Copy link

Pilad commented Mar 14, 2024

@Pilad Can you confirm that with the latest nightly?

2.6.0 - 389
изображение
IronChest-6.0.82

@Caedis
Copy link
Member Author

Caedis commented Mar 14, 2024

Did you spawn that in manually?
Is there a recipe?

@Pilad
Copy link

Pilad commented Mar 14, 2024

Did you spawn that in manually? Is there a recipe?

Place iron chest . Click upgrade iron to silver. Try open gui.

@Caedis
Copy link
Member Author

Caedis commented Mar 14, 2024

I noticed the issue and created a PR for it. Turns out whoever added the iron to silver upgrade forgot to add it to a list.

@Pilad
Copy link

Pilad commented Mar 14, 2024

I noticed the issue and created a PR for it. Turns out whoever added the iron to silver upgrade forgot to add it to a list.

Thanks for fixing the problem.

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.

5 participants