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

About shrinking whynot_compat this mod #59

Open
3 of 6 tasks
bell07 opened this issue Feb 13, 2022 · 9 comments
Open
3 of 6 tasks

About shrinking whynot_compat this mod #59

bell07 opened this issue Feb 13, 2022 · 9 comments

Comments

@bell07
Copy link
Collaborator

bell07 commented Feb 13, 2022

Issue 1 from previous whynot_compat repo. Issue Author: @Lazerbeak12345

I was wondering about this:

What if for every part of this mod (mwc) we make either an Issue or PR for the related mod(s) that would need to be changed to make this mod (mwc) not need those lines of code anymore?

So that'd be an issue or PR for every line in ./depends.txt. PRs would be more likely to be merged.

Granted I think even if completely empty this mod should still exist. There's still more incompatibilities between mods I've seen in Whynot, unfortunately, and of course many more to come as WN expands. I'd imagine that this would serve as a temporary holding place for compatibility code with the assumption that not a single line of said code will remain in this mod for very long.

I have seen actually a few references to such attempts in this code. Try-try again I say. If one mod won't accept the PR, then the other one Targetermight (especially if the former is linked).


@bell07
Copy link
Collaborator Author

bell07 commented Feb 14, 2022

@Lazerbeak12345 , I did some optimizations after whynot_compat was moved into game.

Checked all entries if still relevant. Most of them still valid, caused by homedecor recipes :-(

The farming part may can be removed because the migration away the farming_plus is years ago.

Is still any todo from your point of view or can this issue be closed?

@Lazerbeak12345
Copy link
Collaborator

I think we should make new issues for each specific "entry" and close this, more general one. This will allow us to address the still-important problem that we have anything in this unfortunate (and yet appreciated - at least by me) mod in the first place.

Not to say you have to write the code to fix these problems - if I feel up to it I might even take care of some of that, if others don't when I bring it up on the appropriate repos (either via a PR or an issue :) ). It's just important that we have an issue made (on our repo at first) for each of these needs the mod is addressing.

@bell07
Copy link
Collaborator Author

bell07 commented Feb 15, 2022

The most issues I do not know how to address. The kinds are

  1. Some mods (homedecor) uses items in their recipes from mods not in whynot (cottages, moreblocks). The aliases replace the items in recipes by comparable items from mods in whynot.

  2. Some mods provides the same item, like "mtfoods:sugar" vs "farming:sugar". Remove one and alias to other unify the item.

  3. Recpie change because of overlapping recipes

  4. Compatibility for old worlds after mod replacement in game, like farming_plus or endless_apples replacement.

  5. Missed recipes. Ok, missed recipes can be proposed to upstream

@Lazerbeak12345
Copy link
Collaborator

TLDR I think everything can be proposed to upstream.

  1. With number 1, we need to make an issue on their repo. Mods should never use items from another mod that they don't depend on - unless that dependency is optional (ex moarmor doesn't add waffle armor if the waffles mod isn't there).
  2. With number 2, we need to make an issue in Farming Redo to fix that most likely - but if they don't want to fix it, then we should then try making an issue in MTFoods.
  3. Related to others.
  4. Needs specific issue for each mod replacement.
  5. You got it :)

@bell07
Copy link
Collaborator Author

bell07 commented Feb 16, 2022

https://github.com/minetest-mods/homedecor_modpack/pull/402 - Rejected, does not exists since the repo is moved to gitlab and back to github
https://github.com/tenplus1/farming/pull/39 - Rejected. Repo does not exists anymore
rubenwardy/food#32 Ignored since 2017. Repository archived

I given up. Maybe you have more luck

@Lazerbeak12345
Copy link
Collaborator

For future reference here's the new repo for each of these

As for food: that's not the right place to send the pull request. That's like (please excuse my attempt at comedy) asking Julius Caesar to fix his gosh darn crumbling road when he's been dead for a few thousand years. I think that PR needs to go to Farming Redo (as farming from MTG is not maintained).

Lets make a new issue (on this repo) if the issue doesn't already exist for each of these things. If I have time (and I remember) I might do this myself. Once we have the issue made here we can backlink to it, so they know it's not just "I tried this and it didn't work" and they know its "you have broken my game" - something I imagine might be taken much more seriously.

@Lazerbeak12345
Copy link
Collaborator

For the home decor things, I think we need to make an issue to remove homedecor entirely. I suspect it breaks most whynot rules.

@Lazerbeak12345
Copy link
Collaborator

Checkboxes added to OP

@Lazerbeak12345
Copy link
Collaborator

updated issue for farming plus aliases completion in #97

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

No branches or pull requests

2 participants