Skip to content

Comments

Add switch for partial recipes data warning#1881

Open
MoonWX wants to merge 3 commits intoshedaniel:19.x-1.21.5from
MoonWX:19.x-1.21.5
Open

Add switch for partial recipes data warning#1881
MoonWX wants to merge 3 commits intoshedaniel:19.x-1.21.5from
MoonWX:19.x-1.21.5

Conversation

@MoonWX
Copy link

@MoonWX MoonWX commented May 1, 2025

#1866 #1844
I added a switch that allows users to choose whether to disable the partial recipes warning.
I'm uncertain about the placement of this switch. Please let me know if it should be moved or if you have suggestions for a better location.
Screenshot_2025-05-02_07-17-31
Screenshot_2025-05-02_07-05-44

@MoonWX
Copy link
Author

MoonWX commented May 1, 2025

In case the author doesn't review the PR for a while, I've prepared a pre-compiled version for download.
RoughlyEnoughItems-19.0.9999.zip

"Do not show again" button sets the REI config option implemented by @MoonWX
@Euheimr
Copy link

Euheimr commented May 16, 2025

In case the author doesn't review the PR for a while, I've prepared a pre-compiled version for download. RoughlyEnoughItems-19.0.9999.zip

@MoonWX I really liked your implementation, so I added to your commit with #1891 !

Not sure how you packaged it. If you could advise me how you did that I'd be happy to repackage it and upload here like you did.

Edit: figured it out
RoughlyEnoughItems-19.0.9999_PR-#1891.zip

@MoonWX
Copy link
Author

MoonWX commented May 26, 2025

Apologies for the force push. My intention when rebasing @Euheimr's changes onto my branch was primarily to save them the effort of handling the merge and potential conflicts themselves – I wanted to take care of the integration part. (my apologies for the clumsy force push that resulted!)

That said, the main goal is simply to get our combined work merged smoothly. So, if @Euheimr would prefer the approach where they merge my latest changes into their PR, that's perfectly fine with me. Just let me know, and I'll immediately close this PR to keep things clear and ensure we have a single, focused Pull Request for this feature. What matters most is finding the easiest path forward!


@Euheimr Thanks for the positive feedback on my implementation! I'm glad you found it useful.

Looking back, I noticed a few areas that weren't quite right and could use some improvement.

For instance, I realized I'd misplaced the partial_recipes_warning setting. I initially put it under advanced.tooltips, but it logically belongs under advanced since it's not strictly a tooltip setting. This misplacement also affected the corresponding entries in the language files:

-"config.rei.options.appearance.partial_recipes_warning": "Partial Recipes Warning",
-"config.rei.options.appearance.partial_recipes_warning.desc": "Whether to show partial recipes warning or not.",
 "config.rei.options.groups.appearance.advanced": "Advanced",
+"config.rei.options.appearance.partial_recipes_warning": "Partial Recipes Warning",
+"config.rei.options.appearance.partial_recipes_warning.desc": "Whether to show partial recipes warning or not.",
 "config.rei.options.appearance.rainbow": "Rainbow o(〒﹏〒)o",

These should definitely be grouped under the advanced section for better organization.

I've pushed a commit to correct these issues. Please feel free to review and merge these changes into your fork if you like.

@Euheimr
Copy link

Euheimr commented May 26, 2025

That's totally fine with me especially considering my changes relied on your changes anyway.

I will close my PR ( #1891 ) with the context that it is being added here with your PR. Thank you!


Edit: @MoonWX if you would like to use any photos or information provided in my (now) closed PR #1891 you have my permission to use anything you need from there

Additionally, I compiled your last commit 100b596 for download here: RoughlyEnoughItems-19.0.9999_100b596.zip

@MoonWX
Copy link
Author

MoonWX commented May 26, 2025

@Euheimr Thanks for your cooperation! To be honest, I had absolutely no idea how to add this button myself, so arguably, your contribution is actually greater. And since your PR was newer, logically speaking, yours should have been the one to stay. The main reason I merged it directly into my PR was that I was afraid of troubling you and I didn't have any other intentions. Thanks again for your understanding!

I hope the author(s) can merge this PR soon, as many people need it. Thanks for all your effort!

@Tom1817
Copy link

Tom1817 commented May 28, 2025

Hey just wanted to thank you both so much for your work on this, I've been keeping an eye on it but not checked in for a while until i saw the email - thank you!

MoYuan-CN added a commit to mc521-lab/RoughlyEnoughItems that referenced this pull request Sep 6, 2025
Maually merge shedaniel#1881 (19.x-1.21.5) to 20.x-1.21.6
Just for self-compiling usage. It may not working.
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.

4 participants