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]: luscious #80

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

[Mod Request]: luscious #80

Lazerbeak12345 opened this issue Apr 3, 2022 · 7 comments
Labels
breaks-rule3 This mod currently destroys the world (read: player's work) breaks-rule4 This mod currently has bad code quality. breaks-rule9 This mod is currently incompatible with other whynot mods. breaks-rule10 This code or artwork is not under a compatible licence 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://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.
    • Uses the deprecated depends.txt Mods missing mod.conf are deprecated #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 Memory cache + Biome API sofar/luscious#6 for the fix of almost all of this
  10. Licence
    • Licence is not documented (reported)

Other comments

This was split from #32


Upstream issues reported and pulls requested so far:

@Lazerbeak12345 Lazerbeak12345 added new-mod-proposal Consider the mod can be applied to the game breaks-rule1 This mod currently takes over the game. breaks-rule3 This mod currently destroys the world (read: player's work) mapgen New mod proposal affects mapgen labels Apr 3, 2022
@Lazerbeak12345 Lazerbeak12345 mentioned this issue Apr 5, 2022
4 tasks
@Lazerbeak12345
Copy link
Collaborator Author

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 Lazerbeak12345 added the breaks-rule9 This mod is currently incompatible with other whynot mods. label May 30, 2022
@Lazerbeak12345
Copy link
Collaborator Author

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
Copy link
Collaborator

dacmot commented Jun 12, 2022

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
Copy link
Collaborator Author

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 Lazerbeak12345 removed the breaks-rule1 This mod currently takes over the game. label Aug 7, 2022
@Lazerbeak12345
Copy link
Collaborator Author

This PR has some work towards fixing that hack. sofar/luscious#6

@Lazerbeak12345 Lazerbeak12345 moved this to Report Issue(s) upstream in Add New Game Content Dec 16, 2022
@Lazerbeak12345 Lazerbeak12345 added breaks-rule4 This mod currently has bad code quality. breaks-rule10 This code or artwork is not under a compatible licence labels Nov 1, 2023
@Lazerbeak12345
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Reported issues upstream

@Lazerbeak12345 Lazerbeak12345 moved this from Report Issue(s) upstream to Awaiting Upstream Issues in Add New Game Content Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-rule3 This mod currently destroys the world (read: player's work) breaks-rule4 This mod currently has bad code quality. breaks-rule9 This mod is currently incompatible with other whynot mods. breaks-rule10 This code or artwork is not under a compatible licence mapgen New mod proposal affects mapgen new-mod-proposal Consider the mod can be applied to the game
Projects
Status: Awaiting Upstream Issues
Development

No branches or pull requests

2 participants