minetest-whynot / whynot-game

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

Moreblocks https://github.com/minetest-mods/moreblocks/ #25

Open BruceMustache opened 4 years ago

BruceMustache commented 4 years ago

Moreblocks is a nice mod for decoration that you can do so many things if you're creative forum: https://forum.minetest.net/viewtopic.php?t=509 video: https://youtu.be/dr-a_cLlA8c

bell07 commented 4 years ago

I considered the Moreblocks some times ago and decided against it. The mod registers a lot of decorative nodes, accessible by circular saw instead of crafting or just without recipe. Even you do not use the registered blocks, the game performance decreases. The mod does redefinitions of items from other mods like change recipe for sign_wall. So the "why not" answer in the past was "because the mod take over the server".

But I need to reconsider the mod, my last test was years ago.

BruceMustache commented 4 years ago

I recommend to use the set the option moreblocks.stairsplus_in_creative_inventory = false To hide the extra nodes. I prefer that and just use the circular saw

Lazerbeak12345 commented 2 years ago

Ok, so I thought I'd push this through and do a review. "TODO" checkboxes are problems I found with this mod.

Review

  1. Take over game (no way to opt-in or out)?
    • It does add a few extra crafting recipies - but it doesn't remove any. (default:sapling -> default:stick is one such example) Not an issue, in general. Issues for specifics added.
    • [ ] It changes lots of recipes.
    • [ ] Tool repair buff changed from 2% to 10% (see below in code review section)
  2. Server strain?
    • Perhaps? aliases.lua has a lot of LBMs. I'm not familiar enough with them to know if this is too much or would cause lag.
  3. Destroy Player work?
    • No. Not an issue.
  4. Code quality?
    • init.lua is good
    • crafting.lua is good
    • config.lua is good
    • circular_saw.lua is good
    • stairsplus/init.lua is good
    • stairsplus/defs.lua is good
    • stairsplus/recipes.lua is okay
    • stairsplus/common.lua is okay
    • stairsplus/stairs.lua is good
    • stairsplus/slabs.lua is good
    • stairsplus/slopes.lua is good
    • stairsplus/panels.lua is good
    • stairsplus/microblocks.lua is good
    • stairsplus/custom.lua is good
    • stairsplus/registrations.lua is okay (and provides thorough compatibility with stairs) Only complaint is that it adds to the default:* domain.
    • nodes.lua is good
    • redefinitions.lua has ... problems.
      • [ ] https://github.com/minetest-mods/moreblocks/issues/192 Changes quantity of some crafts to relative values. (not future compatible)
      • Changes quantity of default:sign_wall craft to old + 1
      • Changes quantity of default:paper craft to old * 4
      • Changes quantity of (re) carts:.*rail and default:.*rail craft to old + old/2
      • [ ] Changes global tool repair buff from 2% to 10%
    • aliases.lua has a fair bit more than just aliases.
  5. All-in-one?
    • [ ] It's big, but I got through it. It does seem bloated, lots of things could be broken out to their own mod - and can still be backwards compatible. (stairsplus/ especially, circular_saw.lua would need to be its own mod for that to work.)
  6. Survival-friendly?
    • Yes. Not an issue.
  7. "Cheats" available to non-moderators?
    • [x] 4 stick (this mod adds) -> 1 wood -> 4 planks -> 4*4 sticks. This is unbalanced. You can turn 1 wood into infinite wood. Are there more of these?? (I might have missed another such imbalance)
  8. Version control?
    • Yes, github. Not an issue.

Setting changes

I personally reccomend that these settings apply:

Other notes

I'll add the tags that seem to fit the most.

Lazerbeak12345 commented 2 years ago

Not sure about rule 2

Lazerbeak12345 commented 2 years ago

All of these should be fixable. If they like it that way, we can PR them a change to add a setting, and specify a different value in whynot game, with the default equal to current behavior. If they don't like it that way, it can be changed.

Best to have one PR per bullet "todo" point above.

Lazerbeak12345 commented 2 years ago

The stick thing was a misunderstanding. No infinite wood, game is not unbalanced.

bell07 commented 2 years ago

About 2: lbm's are ok. LBM's are called on map load only. The ABM for horizontal_tree should be LBM too. ABM are called each globalstep so it drain the server. But one ABM is no ABM ;-)

bell07 commented 2 years ago

What is the decision? Should this issue just be closed because of all breaks?

Lazerbeak12345 commented 2 years ago

Well, here's my thoughts. We should make issues for each concern in moreblock's repo, and tie them back here. Should they choose to fix moreblocks, then we can add it.

dacmot commented 2 years ago

I'm not a big fan of the 100 pages of stairs... is moreblocks of any use without stairsplus?

Overall, the value-added isn't convincing enough for me to say we should include it.

bell07 commented 2 years ago

The 100 pages in creative and crafting-guide can be hidden by setting moreblocks.stairsplus_in_creative_inventory in minetest.conf.

The circular saw, give you access to all the new nodes, so basically rule 1 is not violated.

I used moreblocks some years ago, the mayor issue with them is: If you are able to build 1/16 blocks, it is own decorative style, that breaks the default blocky style 1 block (full node) - 1/2 blocks (slabs). I removed it from my collection to to save the blocky style ;-)

If we plan to add moreblocks to whynot, we need to consider existing mods that have optionally moreblock support, if it is still ok. Also we need to remove some comatiblity aliases from whynot_compat

Lazerbeak12345 commented 2 years ago

Admittedly, I'm still worried about the other two game balancing things, but perhaps it's fine.

Lazerbeak12345 commented 2 years ago

Their pr https://github.com/minetest-mods/moreblocks/pull/191 seems to do a lot, might fix more than just 2 of the concerns i've had with this mod

Things that PR would fix that I've highlighted in my review above:


bell07 commented 2 years ago

If moreblocks is splitted in multiple mods, we need to consider each of them separatelly for whynot

Lazerbeak12345 commented 1 year ago

Adding back rule 1, we have some checkboxes I'm worried about. Mostly the buff changes. Bell was mostly talking about the new nodes.