minetest-whynot / whynot-game

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

change to sfence clothing mod fork #61

Open bell07 opened 2 years ago

bell07 commented 2 years ago

sfence created a fork of clothing mod and did a lot of changes.

https://github.com/sfence/clothing/tree/v2

If we like to change to this mod, we need to reconsider if this fork still match the whynot rules

Lazerbeak12345 commented 2 years ago

edit to above: linked to git diff

Lazerbeak12345 commented 2 years ago

List of code changes from that diff:

Lazerbeak12345 commented 2 years ago

I may have missed some, feel free to comment or edit to add them.

Lazerbeak12345 commented 2 years ago

Perhaps we should have a goal/rule for Whynot to fit in a ranking within (at least one) specific game rating system. It would have to be unofficially, unless we know of a way to get it officially rated that is free. I'll make this comment as an issue at some later time, if there's more push to get this sfence fork into whynot.

Lazerbeak12345 commented 2 years ago

A note: the questionable thing I'm referencing can be disabled via a setting.

dacmot commented 2 years ago

Added mod.conf

Lazerbeak12345 commented 8 months ago

I think many of the checkboxes I pointed out might not be enough of problems to block. So long as we change that one setting I mentioned, we could move forward with this.

Lazerbeak12345 commented 7 months ago

rule review, since we somehow missed doing this:

  1. don't take over the game
    • can't
  2. don't take over the server
    • doesn't
  3. don't destroy the world
    • won't
  4. no bad code quality
    • circular dependency with skinsdb
  5. no all-in-one mods or lots of unused dependencies
    • dependencies could use some cleanup
  6. most items obtainable in survival
    • yes
  7. No cheating mods
    • purely esthetic
  8. Mod under version control
    • yes (git)
  9. Mod must not be incompatible with other mods
    • no known incompatibilities
  10. The code and artwork for the mod must be under a compatible licence
    • LGPLv2.1 for code, cc by sa 3 for some media, cc0 for other media, cc by sa 4 for other media
bell07 commented 7 months ago

Looked into new skinsdb support now: 2 new skins are added: Male and female in underwear. Parameter to set the male to the default skin The parameter overrides the default skin texture. I proposed the change to proper replace the default skin: https://github.com/sfence/clothing/issues/13

sfence commented 7 months ago
* Dye machines, looms and the spinning machine are using seperate `.obj` files when a single key-framed .obj would work instead

I use generated .obj from Minetest schematic in .we format. I don't know how to do key-framed .obj files But I am open to updating it if someone brings a better 3D model.

* Spinnning machine is using attached nodes when a single animated `.obj` file would work instead

What do you mean by attached nodes?

* [ ]  Uses generic appliances api?  Undocumented dep?

Should be documented now, see https://github.com/sfence/appliances/blob/master/API.md

dacmot commented 7 months ago

Updated rules check

dacmot commented 7 months ago

I don't like the new skinsdb dependency. It creates a circular one because skinsdb also depends on clothing. As a result, skinsdb can't be listed in optional dependencies.

It looks to me like the dependency on skinsdb in this mod is extraneous. Clothing is about adding clothes, not skins. If people want a naked skin, they should be able to use one, sure. But I don't think it should be the clothing mod's job to make it available.

Originally posted by @dacmot in https://github.com/sfence/clothing/issues/14#issuecomment-1995925652