minetest-whynot / whynot-game

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

mobs_animal #16

Open raistlin opened 4 years ago

raistlin commented 4 years ago

I'm wondering why mobs_animal is not already included.

https://notabug.org/TenPlus1/mobs_animal

bell07 commented 4 years ago

The reason is if mod is enabled the mobs take over the world. It is not possible to get mobs-free areas. I played some time with the mod. If you are at home and takes longer time for testing crafting recipes, going outside the mobs are everywhere. Still waiting for better spawning algorithm for peaceful mobs. The mobs_monster are in game because you are able to get mobs-free areas by placing light.

raistlin commented 4 years ago

Ok, I'll close the issue then until mobs_animal is good enought.

Thanks

Lazerbeak12345 commented 2 years ago

Request

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

https://codeberg.org/tenplus1/mobs_animal

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?
    • (When last checked by @bell07 in this issue) It might not be possible to get mob free spawns
    • Supports custom spawn algorithm using a spawn.lua file inside the mod folder.
  2. When could this mod be a strain on the server when no players are using the mod?
    • Performance could be an issue, but I haven't tested it. I'd bet it's not.
  3. When does this mod destroy player's work?
    • Never.
  4. Have you looked at the code? If so, what stood out as things that might need fixing?
    • init.lua
    • chicken.lua
      • [x] https://notabug.org/TenPlus1/mobs_animal/pulls/39 indentation formatting issues makes it harder to read
      • [ ] If ethereal is present, chicken will not spawn on default:dirt_with_grass
      • [ ] Reserves node name :mobs:egg when :mobs_animal:egg would have been clearer.
      • [ ] Reserves craftitem name :mobs:chicken_egg_fried when :mobs_animal:chicken_egg_fried would have been clearer.
      • [ ] Reserves craftitem name :mobs:chicken_raw when :mobs_animal:chicken_raw would have been clearer.
      • [ ] Reserves craftitem name :mobs:chicken_cooked when :mobs_animal:chicken_cooked would have been clearer.
      • [ ] Reserves craftitem name :mobs:chicken_feather when :mobs_animal:chicken_feather would have been clearer.
    • cow.lua
      • [ ] Reserves craftitem name :mobs:bucket_milk when :mobs_animal:bucket_milk would have been clearer.
      • bucket would be very easy to change into an optional dependency.
      • [ ] Reserves craftitem name :mobs:glass_milk when :mobs_animal:glass_milk would have been clearer.
      • vessels would be very easy to change into an optional dependency.
      • [ ] The craft for turning group:food_milk_glass es and a bucket into :mobs:bucket_milk might have subtle balancing issues with outputting only one group:food_mlik_glass (note the falback to vessels:drinking_glass 4)
      • [ ] Reserves craftitem name :mobs:butter when :mobs_animal:butter or :mobs_animal:cow_butter would have been clearer.
      • [ ] Reserves craftitem name :mobs:cheese when :mobs_animal:cheese, :mobs_animal:cow_cheese, :mobs_animal:cheese_wedge or :mobs_animal:cow_cheese_wedge would have been clearer.
      • [ ] Reserves craftitem name :mobs:cheeseblock when :mobs_animal:cheeseblock or :mobs_animal:cow_cheeseblock would have been clearer.
      • [ ] Craft recepie for :mobs:cheeseblock uses group:cheese, forcing :mobs:cheeseblock from being within the cheese group, and making it harder for someone to make a "more_cheese" mod (ex: cheese from goats) by reducing all types of cheese blocks down to :mobs:cheeseblock then back to :mobs:cheese.
    • rat.lua
      • [ ] Unused local rat_spawn
      • [ ] Reserves craftitem name :mobs:rat_cooked when :mobs_animal:rat_cooked would have been clearer.
    • sheep.lua
      • [ ] Reserves craftitem name :mobs:mutton_raw when :mobs_animal:mutton_raw would have been clearer.
      • [ ] Reserves craftitem name :mobs:mutton_cooked when :mobs_animal:mutton_cooked would have been clearer.
    • warthog.lua
      • If ethereal mod is present mobs_animal:pumba does not spawn on default:dirt_with_grass
      • [ ] Unmarked optional dependency ethereal
      • [ ] Reserves craftitem name :mobs:pork_raw when :mobs_animal:pork_raw would have been clearer.
      • [ ] Reserves craftitem name :mobs:pork_cooked when :mobs_animal:pork_cooked would have been clearer.
    • bee.lua
      • [ ] Reserves craftitem name :mobs:honey when :mobs_animal:honey would have been clearer.
      • [ ] Reserves craftitem name :mobs:beehive when :mobs_animal:beehive would have been clearer.
      • It's a little odd that you make a beehive out of three bees....
      • [ ] Reserves craftitem name :mobs:honey_block when :mobs_animal:honey would have been clearer.
    • bunny.lua
      • If mod ethereal present, does not spawn on default:dirt_with_grass
      • [ ] Unmarked optional dependency ethereal
      • [ ] Reserves craftitem name :mobs:rabbit_raw when :mobs_animal:rabbit_raw would have been clearer.
      • [ ] Reserves craftitem name :mobs:rabbit_cooked when :mobs_animal:rabbit_cooked would have been clearer.
      • [ ] Reserves craftitem name :mobs:rabbit_hide when :mobs_animal:rabbit_hide would have been clearer.
    • kitten.lua
      • [x] Indention issues.
      • If mod ethereal present, does not spawn on default:dirt_with_grass
      • [ ] Reserves craftitem name :mobs:hairball when :mobs_animal:hairball would have been clearer.
      • [ ] :mobs:hairball Can turn into a lot of different items, many of them valuable. This can break game balancing.
      • Nothing (chance is 5 times more likely than any other individual item)
      • default:stick
      • default:coal_lump
      • default:dry_shrub
      • flowers:rose
      • mobs_animal:rat
      • default:grass_1
      • farming:seed_wheat
      • dye:green
      • farming:seed_cotton
      • default:flint
      • default:sapling
      • dye:white
      • default:clay_lump
      • default:paper
      • default:dry_grass_1
      • dye:red
      • farming:string
      • mobs:chicken_feather
      • default:acacia_bush_sapling
      • default:bush_sapling
      • default:copper_lump
      • default:iron_lump
      • dye:black
      • dye:brown
      • default:obsidian_shard
      • default:tin_lump
      • [ ] Unmarked optional dependencies:
      • dye
      • farming
      • flowers
      • ethereal
    • penguin.lua
      • [x] Indentation issues
      • [ ] Unmarked optional dependency xocean
    • panda.lua
      • [x] Indentation issues
      • [ ] Panda only spawns if ethereal mod is present.
      • [ ] Unmarked optional dependency ethereal
    • spawn_example.lua
      • [ ] When comments are removed from example, missing endquote on line 10
      • [ ] Do we really need to document an api provided by a different library? (mobs)
    • lucky_block.lua had no 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")
    • Adds a huge amount of animals. Split up into category or something.
  6. Is this mod survival freindly? What items that it provides that should be craftable/obtainable, but arent?
    • Yes. From what I've seen, none.
  7. When does this mod feel like cheating?
    • [ ] :mobs:hairball Can turn into a lot of different items, many of them valuable. This can break game balancing. See above section on kitten.lua file.
  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

The styles don't match, and the sheep's texture is nightmare fuel.

In general, the mob behavior is "passable" quality, but not good.

I haven't tested this thoroughly, I'm just splitting out issue #21 to be easier to assess.

Lazerbeak12345 commented 2 years ago

As mentioned in #21, the only blocker for this for now is determining what settings to use, and if they are good enough

Lazerbeak12345 commented 2 years ago

See above in-depth review to see why rules 4 and 7 are broken.

Each checkbox is something I think we should require to improove before we can accept this mod. One might view it as a checklist for how to make this mod conform.

As for rule 1: as for the mobs mod, the best way to prevent, say, sheep, from spawning is to ensure that there is no default:dirt_with_grass or ethereal:green_dirt where you do not want them to spawn.

Lazerbeak12345 commented 2 years ago

Sent out a PR (linked above) to them to do these things:

  1. Test if they are willing to accept PRs
  2. Start work on fixing the problems found in this review.
  3. Make the maintainer aware of our reservations about this mod, should they choose to dig a bit deeper.
Lazerbeak12345 commented 2 years ago

They've accepted the PR. Marked checkbox accordingly.

Lazerbeak12345 commented 2 years ago

Sent the 2nd pr. I don't think the intlib exposure is that big of a deal. It's no global variable, and it's well named.

Lazerbeak12345 commented 2 years ago

A commit was made that fixes the concerns of the 2nd pr, and some

Lazerbeak12345 commented 1 year ago

Licence is fully compatible.

Lazerbeak12345 commented 6 months ago

Link outdated

Lazerbeak12345 commented 6 months ago

updated link