search

minetest-whynot / whynot-game

Minetest game in minetest mods collection style
GNU General Public License v3.0
14 stars 7 forks source link

[Mod Request]: `luscious` #80

Open Lazerbeak12345 opened 2 years ago

Lazerbeak12345 commented 2 years ago

Request

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

https://github.com/sofar/luscious

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?
    • Takes over the world - but it's usually subtle. (Decorative only)
  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?
    • Doesn't destroy chunks - but does change biomes. Like in 2b2t updates, you might have a winter conifer forest before this mod is present, but after introduction of this mod, the placed blocks are the same, but look different. In my example, the biome appeared to have changed to a savanna plains. This might also only be a cosmetic change, as it was still snowing, and further exploration reveals that the area around this biome was still the same type of forest - but done in a different way.
  4. Have you looked at the code? If so, what stood out as things that might need fixing?
    • Variable names could be better throughout.
    • I don't understand why they say in the readme that this mod can't be used in an existing world. It looks fine to me tbh.
    • [x] Uses the deprecated depends.txt #132 (reported)
  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 simple enough with a playtest.
  6. Is this mod survival freindly? What items that it provides that should be craftable/obtainable, but arent?
    • Yes. No issues with crafting.
  7. When does this mod feel like cheating?
    • It doesn't. (Though the chances of farming:* items seem to be higher in a wider variety of biomes)
  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?
    • [ ] Error message 2022-04-29 18:03:41: ERROR[Server]: unable to find map for 140739635806206 while walking around in world upgraded to this mod. (there was two of these as I explored, different number for the second one)
      • From their README: (bold added by me) 'Installation: Create a new world, enable this mod. Do not use this mod in an existing world, ever.'
      • Biomes in old chunks seem to have changed.
      • I'm unsure actually. It's still snowing?
      • See https://github.com/sofar/luscious/pull/6 for the fix of almost all of this
  10. Licence
    • [x] Licence is not documented (reported)

Other comments

This was split from #32

Upstream issues reported and pulls requested so far:

Lazerbeak12345 commented 2 years ago

Any second opinions on if this qualifies as breaking rule 3 still? I'm unsure, but if not for the upgrade bugs, I'd say no.

Lazerbeak12345 commented 2 years ago

I'm fairly confidant, after reading the code, that this mod has no legit reason to say that it can't be added to an existing world.

dacmot commented 2 years ago

Just tried this mod on a new world. I like it very much.

Any second opinions on if this qualifies as breaking rule 3 still? I'm unsure, but if not for the upgrade bugs, I'd say no.

It seems to make transitions between biomes a bit less on/off. I think modifying biomes, without modifying nodes, at edge transitions ok for rule 3.

As for rule 1, it would argue it doesn't take over the world. Aside from the minimal biome changes, the rest is just cosmetic (different shades of green...)

Rule 9 does worry me a bit. I haven't encountered any error on a new world. If modifying an existing one causes errors, that would be a show stopper. There's also the two supposed "bugs" with leaf decay and snow pine trees. Leaf decay seemed ok to me, and I did encounter pine trees covered in snow... so I don't know what that's about. Finally there's the code using an old hack which could possibly be simplified using the 5.0 get_biome_data() forum, lua_api ... though if it has no impact on performance and stability should be OK.

Lazerbeak12345 commented 2 years ago

The errors seemed to me to just be red text IE: it doesn't actually cause any real issues. The message is just saying that there's no way to get biome data on a chunk that wasn't loaded with this mod installed.

I think we should for sure send them a PR to move away from their old hack (which is the cause of the error message) and to move towards get_biome_data. (I had no idea that was a function till you mentioned it btw)

Lazerbeak12345 commented 1 year ago

This PR has some work towards fixing that hack. https://github.com/sofar/luscious/pull/6

Lazerbeak12345 commented 8 months ago

Updated OP with more information. The license info is invalid, which needs to be reported upstream, mod.conf issue needs to be reported as well.

Lazerbeak12345 commented 1 month ago

Reported issues upstream