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

[Mod Request]: underch #81

Closed
7 tasks
Tracked by #32
Lazerbeak12345 opened this issue Apr 3, 2022 · 8 comments
Closed
7 tasks
Tracked by #32

[Mod Request]: underch #81

Lazerbeak12345 opened this issue Apr 3, 2022 · 8 comments
Labels
breaks-rule1 This mod currently takes over the game. breaks-rule4 This mod currently has bad code quality. breaks-rule5 This mod is currently an all-in one or has a lot of dependencies not already in WN. mapgen New mod proposal affects mapgen new-mod-proposal Consider the mod can be applied to the game

Comments

@Lazerbeak12345
Copy link
Collaborator

Lazerbeak12345 commented Apr 3, 2022

Request

This is the URL for the mod I would like to be added:

https://gitlab.com/h2mm/underch

My opinion on how it fits with the whynot rules:

Refer to the Whynot Readme for full rule descriptions/reasons.

  1. In what ways might this mod take over the game?
    • default:stone seems to be very rare now. Mostly everything underground is from underch.
    • I'm not sure, but it seems like new ore registration might not be possible while this mod is present. It adds support for some new ore generators, and even provides an API for other ore generators to use. If a mod is using, say the instant_ores mod, would those ores show up here? TODO test rarity of ores that this mod is unaware of (that doesn't also add support manually) in a world with and without this mod.
  2. When could this mod be a strain on the server when no players are using the mod?
    • Never.
  3. When does this mod destroy player's work?
    • It doesn't.
  4. Have you looked at the code? If so, what stood out as things that might need fixing?
    • init.lua:13 If the global stairs is absent this will throw an error. Not a huge deal since it usually is if default - a marked dependency - is present. (they still check in case stairs is absent though, so they might like this change)
    • functions.lua:13 (Unless I'm mistaken) moreblocks adds more types of stairs than stairsplus. It might be a good idea to prefer moreblocks - but the catch is backwards compatibility.
    • jit.lua:573 I forget if this is a thing, but would it be better to get the list of ores from the minetest engine instead of manually listing them upon mod detection?
    • init.lua:120 this might be duplicating the ABM from default. Does this cause any issues?
  5. In what way might this mod be reduced to be more simple (as in "Keep it Simple Stupid") (ex: "the foobar mod could be made more simple by splitting into two mods, foo and bar")
    • Seems to add a ton of stuff - it would be much better as a modpack. (see nodes.lua)
  6. Is this mod survival freindly? What items that it provides that should be craftable/obtainable, but arent?
    • Yes.
  7. When does this mod feel like cheating?
    • Never.
  8. Does this mod use the software "git" for version control? (note: we are asking about git. Github, Gitlab, notabug and hundreds of other git providers exist but are not specificly needed, although these do qualify).
    • Yes
  9. Upon testing this mod, what errors, odd behavior, or other incompatibilities were noticed?
    • None.

Other comments

This was split from #32

@Lazerbeak12345 Lazerbeak12345 added new-mod-proposal Consider the mod can be applied to the game mapgen New mod proposal affects mapgen breaks-rule1 This mod currently takes over the game. breaks-rule3 This mod currently destroys the world (read: player's work) breaks-rule5 This mod is currently an all-in one or has a lot of dependencies not already in WN. labels Apr 3, 2022
@Lazerbeak12345 Lazerbeak12345 mentioned this issue Apr 5, 2022
4 tasks
@Lazerbeak12345 Lazerbeak12345 removed the breaks-rule3 This mod currently destroys the world (read: player's work) label Apr 30, 2022
@Lazerbeak12345
Copy link
Collaborator Author

Updated after quick playtest with initial underch and post-creation underch.

@Lazerbeak12345
Copy link
Collaborator Author

Started code review then got busy

@Lazerbeak12345 Lazerbeak12345 added the breaks-rule4 This mod currently has bad code quality. label Jun 19, 2022
@Lazerbeak12345
Copy link
Collaborator Author

Finished code review. rule 1 seems to be a real problem with this mod...

@dacmot
Copy link
Collaborator

dacmot commented Jun 21, 2022

@Lazerbeak12345, in your review did you see a way to set altitude/depth limits, preferably in a configuration file? The only parameter exposed in minetest is underch_ores_jit...

Can it be compressed to a small depth interval (say 5000 blocks)?

@dacmot
Copy link
Collaborator

dacmot commented Jun 21, 2022

Just created a new world of valleys with WN and underch. Got nothing but black, as if I had spawned underground. Created a second world and that one was OK.

I'm also a little concerned about the lava in the sky bug (https://content.minetest.net/threads/2845/, https://gitlab.com/h2mm/underch/-/issues/7).

@Lazerbeak12345
Copy link
Collaborator Author

Lazerbeak12345 commented Jun 21, 2022

My largest concern is that it seems like it doesn't allow for any sort of ore-registration for mods that don't know about underch, and underch doesn't know about them. There's a lot of quality mods (many that we don't even have issues for yet) that add extra ores, and have this issue.

@Lazerbeak12345
Copy link
Collaborator Author

@Lazerbeak12345, in your review did you see a way to set altitude/depth limits, preferably in a configuration file? The only parameter exposed in minetest is underch_ores_jit...

Can it be compressed to a small depth interval (say 5000 blocks)?

I didn't see anything like that at all.

@Lazerbeak12345
Copy link
Collaborator Author

I feel like the core goals of this mod conflict with Whynot. I'll close this for now. Feel free to reopen if .... whatever reason really.

@Lazerbeak12345 Lazerbeak12345 reopened this Aug 6, 2023
@Lazerbeak12345 Lazerbeak12345 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-rule1 This mod currently takes over the game. breaks-rule4 This mod currently has bad code quality. breaks-rule5 This mod is currently an all-in one or has a lot of dependencies not already in WN. mapgen New mod proposal affects mapgen new-mod-proposal Consider the mod can be applied to the game
Projects
None yet
Development

No branches or pull requests

2 participants