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

Updated Grab Bags ItemDropDatabase with 1.4.4 Changes #3832

Merged
merged 8 commits into from
Oct 8, 2023

Conversation

Rijam
Copy link
Contributor

@Rijam Rijam commented Sep 23, 2023

What is the bug?

Some of the grab bags were not updated for 1.4.4, this includes:

  • Eater of Worlds Treasure Bag
    • Demonite and Shadow Scale drops were increased. Also, they are increased even more in Master Mode.
  • Brain of Cthulhu Treasure Bag
    • Crimtane and Tissue Sample drops were increased. Also, they are increased even more in Master Mode.
  • Gold and Titanium Crates
    • Adjusted the chances on the Life Crystal and Hardy Saddle.
  • Ice and Boreal Crates
    • Is supposed to drop the Ice Bow in Don't Dig Up worlds instead of the Snowball Cannon.
  • Golden Lock Box
    • Is supposed to drop the Bubble Gun in Don't Dig Up worlds instead of the Aqua Scepter.
    • Treasure Magnet is in its own loot pool.
  • Obsidian Lock Box
    • Is supposed to drop the Unholy Trident in Don't Dig Up worlds instead of the Flower of Fire.
  • Biome Crates dropped one too many Gold Coins (5-13 instead of 5-12).
  • Moon Lord's Treasure Bag was already fixed in a previous PR Fixed Moon Lord Treasure Bag Not Dropping Two Weapons #3814 , but @jjohnsnaill pointed out that it was not the correct solution. Now uses FromOptionsWithoutRepeatsDropRule to not drop the same item twice.

How did you fix the bug?

Adjusted the drops for each of them to match the 1.4.4 changes.

Are there alternatives to your fix?

  • For the Eater of Worlds and Brain of Cthulhu Treasure Bag, instead of having a IsNotMaster and IsMaster drop, there could be a regular Common drop and a IsMaster drop that makes up the difference. I thought that would be less readable, so I didn't do it that way.
  • For the Gold and Titanium Crates, I created a new NotScalingWithLuckWithNumerator so I could get the numerator. I decided to create an entirely new class for it instead of editing the existing NotScalingWithLuck to add the numerator there to reduce on vanilla patches. If there is a way to change the chances to the new ones without adding a new IItemDropRule, suggest it. A new IItemDropRule named NotScalingWithLuckWithNumerator was added with new constructor for CommonDropNotScalingWithLuck. Currently not used by anything, but it is still useful for modders. Let me know if you want the name to be changed.
  • For the Don't Dig Up specific items, I set the hideLootReport to true. It could be false if you feel like it should be shown.
  • For the Moon Lord Treasure Bag, I used new FromOptionsWithoutRepeatsDropRule because it didn't have a rule defined in the ItemDropRule class already. We could add one if we wanted to.

Other Note

  • I renamed a few variables to make them more descriptive.
  • I replaced all of the numerical IDs with full name IDs (1 -> ItemID.IronPickaxe). Chicken Bones said they prefer the full names.

@Rijam Rijam marked this pull request as ready for review September 24, 2023 07:07
@JavidPack JavidPack self-assigned this Sep 25, 2023
@Rijam
Copy link
Contributor Author

Rijam commented Sep 26, 2023

Will investigate soon: I think the biome crates are dropping 5-13 gold coins instead of 5-12. (Main.rand.Next(5, 13) is the vanilla numbers and it was probably mistaken because the max is exclusive.)

@JavidPack
Copy link
Collaborator

You mentioned looking into something, is this PR ready for review right now?

@Rijam
Copy link
Contributor Author

Rijam commented Oct 2, 2023

Sorry I've been super busy IRL so I haven't looked into it yet. John Snail pointed out on #3814 that the implementation for that may need to be changed, too.

@JavidPack
Copy link
Collaborator

Ok, just checking.

@Rijam
Copy link
Contributor Author

Rijam commented Oct 4, 2023

Ok, updated and ready for review now.

@JavidPack
Copy link
Collaborator

Great, working on it.

@JavidPack JavidPack merged commit 24e759c into tModLoader:1.4.4 Oct 8, 2023
JavidPack added a commit that referenced this pull request Oct 18, 2023
* EoW, BoC, Golds, Ices, Lock Boxes

* Better Don't Dig Up Rules

* Magic numbers to full names

* Travelling Merchant with two Ls' items

* Banished NotScalingWithLuckWithNumerator Class

* NotScalingWithLuckWithNumerator returns

* Moon Lord Bag and Biome Crate Coins

* Fix other inconsistencies.

---------

Co-authored-by: JavidPack <javidpack@gmail.com>
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.

None yet

2 participants