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

Assembling Machine recipes for Lootbag upgrading and downgrading #654

Merged
merged 5 commits into from
Jul 1, 2023
Merged

Assembling Machine recipes for Lootbag upgrading and downgrading #654

merged 5 commits into from
Jul 1, 2023

Conversation

Cleptomania
Copy link
Member

This adds recipes as discussed in GTNewHorizons/GT-New-Horizons-Modpack#11861

In general, these are gated by the same tier of assembler that the questbook would gate them by. For example, you need an MV assembler to turn LV lootbags into MV, and an EV to turn HV in to EV.

This is also loosely followed for the other various types of lootbags. For example, the first tier of forestry upgrades in the quest book can be accessed during LV, however you need at least MV tier power/components to get to the next tier of forestry lootbag upgrades based on the quests required, so you need an MV assembler to do that upgrade.

All of these recipes currently use a full amp of power for their respective tier, and take 30 seconds to complete. This of course can be adjusted if it should be different, I just kind of threw these numbers in there.

This also allows downgrading into the previous tier of lootbags via the assembler. Upgrading/downgrading is controlled via circuit configuration.

Copy link
Member

@Dream-Master Dream-Master left a comment

Choose a reason for hiding this comment

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

I am against it. You need no additional item or a enchanted book.

@Dream-Master
Copy link
Member

Dream-Master commented Jun 28, 2023

@Cleptomania ok if you choose only not enchanted lootbags this is ok. But please change the recipe adder to version 2

Example can be found here
https://github.com/GTNewHorizons/NewHorizonsCoreMod/blob/master/src/main/java/com/dreammaster/scripts/ScriptCoreMod.java#L1110-L1115

Copy link
Contributor

@Steelux8 Steelux8 left a comment

Choose a reason for hiding this comment

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

The ability to downgrade lootbags needs some balance-related discussion, regardless of how it's done. Even with just GT tier lootbags, there's already a problem where tiers above LuV would end up rewarding dozens of IV lootbags, and a huge supply of free ender chests, tanks and other stuff. The impact of IV lootbags is already significant enough as it is, and I think we need to talk about whether or not we want to give even more of that free stuff by allowing lootbag downgrades.

@Cleptomania
Copy link
Member Author

Perhaps downgrading can be done at a 1-1 ratio instead of giving back the original 3, such that you can get a lower tier loot bag if you need but you can’t turn higher tier rewards into more loot bags than you originally got.

@Steelux8
Copy link
Contributor

A 1-1 downgrade is not as impactful, but these numbers should still be discussed and agreed on before the PR can be approved, since it's an extra feature on top of the assembler upgrading of lootbags.

@Cleptomania
Copy link
Member Author

I've switched the recipes to use the new builder, as well as went ahead and removed the downgrade recipes, at least until those can be discussed further, so for now it only allows upgrades exactly as the questbook currently does.

@Dream-Master I didn't realize when I made this that the old addAssemblerRecipe and other similar functions were deprecated. Is there are a desire to remove all usage of those from this? Would you want a PR for updating all of the recipes still using that API?

@chochem
Copy link
Member

chochem commented Jun 28, 2023

yea, we are in the process of transforming all those to the new RA2. GT is done. coremod like half-way maybe.

Copy link
Member

@chochem chochem left a comment

Choose a reason for hiding this comment

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

just some more technical points:

@Cleptomania
Copy link
Member Author

I added new changes with the feedback from @chochem

I agree that the AssemblerRecipes is the most sensible place for it, the organization of stuff just wasn't initially clear to me.

I can follow up this PR with a full cleanup of Assembler recipes with the more up to date APIs and conventions(thanks Chochem your previous PR for the chembath stuff is a good frame of reference for that) if no one else is already working on that, but this PR should probably stay separate since it's actually adding new recipes I'd imagine.

@chochem chochem dismissed their stale review June 28, 2023 20:05

all done, thanks

@Dream-Master
Copy link
Member

@Steelux8

@Steelux8 Steelux8 dismissed their stale review June 28, 2023 22:47

Without the downgrade recipes, this is the same process as the questbook, so I have no problems with it.

@Cleptomania
Copy link
Member Author

I saw you pulled in my assembler-cleanup cleanup branch for RA2 conversion into the dev branch. Assuming that’s going to master at some point you could close this PR as that already had these changes applied to it.

@wlhlm
Copy link
Member

wlhlm commented Jun 30, 2023

The dev branch is for making -pre releases for testing changes in nightlies. Accepted changes get merged from PR branch to master/main directly.

@chochem
Copy link
Member

chochem commented Jun 30, 2023

you will need to make a PR for that. and apparently it has some issues and needs some splitting to work.

@Dream-Master Dream-Master merged commit 32d2f86 into GTNewHorizons:master Jul 1, 2023
1 check passed
@Cleptomania Cleptomania deleted the assembler-lootbags branch July 5, 2023 20:24
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