Open Lazerbeak12345 opened 2 years ago
@bell07 could you explain this further?
it is forward compatible, for example as alias support was added from the old stairs nodes to moreblocks https://github.com/mt-mods/homedecor_modpack/blob/master/building_blocks/node_stairs.lua#L9-L12.
ill be frank, mtg stairs mod has a terrible api, you have to pass it parameters rather than a table def it modifies. this means to support it and fix issues/warnings one would need to use the api, then override it, etc. additionally it register the node in its own namespace instead of the mods namespace as moreblocks properly does.
so tldr, there was three options,
option 1 is completely unreasonable from the mess of code and maintainability. additionally following https://github.com/mt-mods/homedecor_modpack/issues/13 we would like to make mods as game agnostic as possible, so keeping a dep around mtg is rather not a good idea.
option 2 (besides reinventing the wheel) was a possibility, however it didnt make the most of sense when we already had a well used community mod for handling stairs
option 3 won out because it allowed us to use a sane api unlike the mtg one, and also provided a nice easy way to handle aliases for the older nodes
Thanks for your insights @wsor4035. @bell07 's in a different timezone from me, so it'll take some time for them to see all this (and those comments made in our issue as well).
Option 3 is no option, since the moreblocks provides a lot of things more, then just stair. I think we need to go with the Option 2. Can you provide a link to the "greek mod" ?
Option 3 is no option, since the moreblocks provides a lot of things more, then just stair. I think we need to go with the Option 2. Can you provide a link to the "greek mod" ?
option 3 is a option and is the chosen option for this repo. you are free to do whatever you like in your own project
I didn't mean to be rude. I meant the moreblock contain a lot more then just some stairs, so it is oversized for my need.
I'd like to ask for more opinions about this issue, so ping @OgelGames and I guess @BuckarooBanzay would be approproate here. There's valid point for complaint in my opinion, main question is should we reopen this issue and look for actual solutions?
While moreblocks is the solution selected by @wsor4035 can it be used only for stairs/slabs without bringing in overrides that have very high potential to break things? (for example https://github.com/minetest-mods/moreblocks/issues/172 and basically no configuration available to fine tune moreblocks content) I think no and it is either everything or nothing.
If moreblocks mod could be made usable for everyone then maybe look at that option and what would need to be changed. Would it be good enough to have configuration to allow disabling parts of moreblocks functionality?
Need to fork moreblocks to make it usable? Sure we could do that too and change a lot while still being able to include future updates from minetest-mods/moreblocks if it seems that they're not accepting changes that would be required.
Can something be done directly in homedecor modpack? Is it possible to drop hard dependency to moreblocks while keeping support for stairs and slabs? Would it be better than working on or if needed forking moreblocks?
Why I do think that issue should be discussed here instead of under minetest-mods/moreblocks repo: Changes that break compatibility happens here, not in moreblocks mod. Discussion should also stay here until there's acceptable solution (which might be just telling people to fuck off if we that's what we choose but I'm not exactly recommending that)
main question is should we reopen this issue and look for actual solutions?
no. see below
While moreblocks is the solution selected by @wsor4035
actually this has already been discussed in the irc/discord by myself and ogelgames and the moreblocks option was choosen. moreover the whole modpack already only supported moreblocks except for the building blocks mod, so precedence was already set.
Would it be good enough to have configuration to allow disabling parts of moreblocks functionality?
no, because its a optional dependency, so literally have the option of just not using it.
Need to fork moreblocks to make it usable? Sure we could do that too and change a lot while still being able to include future updates from minetest-mods/moreblocks if it seems that they're not accepting changes that would be required.
i would say no as its author is still active in the community and decently responsive to prs. however to fork it or not has no bearing here
Can something be done directly in homedecor modpack? Is it possible to drop hard dependency to moreblocks while keeping support for stairs and slabs? Would it be better than working on or if needed forking moreblocks?
this is incorrect. there is NOT a hard dependency on moreblocks just like there never was a hard dependency on stairs, its always been optional
Why I do think that issue should be discussed here instead of under minetest-mods/moreblocks repo: Changes that break compatibility happens here, not in moreblocks mod. Discussion should also stay here until there's acceptable solution (which might be just telling people to fuck off if we that's what we choose but I'm not exactly recommending that)
moreblocks issues can be discussed in there repo and issues here should be discussed here, not sure whats so hard about that. the current solution is we have provided legacy compat for people if they wish (most servers use moreblocks anyways) and people are free if they want to maintain working with the old, terrible, and deprecated(see mtg in maintaince mode and eventually will be removed from mte distribution - which already happens on some distros) they are free to add that themselves. we should not take on the burden of dealing with legacy and ugly code when better/more viable options occur. forcing use of the stairs api is like forcing use of technology from the 1990s when much better apis and support exist elsewhere
Just some thoughts.
In Whynot I re-reviewed the entire moreblocks mod.
I have a detailed list of each thing that I later plan on pr-ing to that mod developer. (sorry, I don’t have the issue number. I’m replying to this via my phone)
Most, if not all, of our (the WhyNot Team’s) concerns about that mod are easily fixed.
In particular, I think moreblocks should be split up into 3 mods:
Of course i’ll bring these points up with them.
Edit: removed email reply junk
In particular, I think moreblocks should be split up into 3 mods:
this is some serious scope creep here talking about moreblocks. anyways, all i have to say on the matter is instead of splitting it into three mods it would be better off with settings to enable/disable parts for legacy compat there
The problem I see here is that the compatibility depends on another mod. While stairs
was an optional mod, the majority of users will have had it enabled, due to it being part of minetest_game
. However, not all users will also have moreblocks
installed.
I completely agree with @Lazerbeak12345 that stairsplus
should be separated from moreblocks
, but that still doesn't solve the problem for users that don't want to install either mod.
I think the only way to solve this is to implement option 1 with a setting to enable it, disabled by default. (something like enable_legacy_stairs
)
I think the only way to solve this is to implement option 1 with a setting to enable it, disabled by default. (something like enable_legacy_stairs)
so your solution is to add a setting that probably no one will know about, and very few if anyone will enable...
if such a thing is decided on, i would propose we enable add a minetest.log statement warning that it will eventually be removed in X amount of time
so your solution is to add a setting that probably no one will know about, and very few if anyone will enable...
If we put it somewhere it can be seen (like in the readme), I'm quite sure those who need it will find it and enable it.
if such a thing is decided on, i would propose we enable add a minetest.log statement warning that it will eventually be removed in X amount of time
Why would it be removed?
See https://github.com/minetest-whynot/whynot-game/issues/49 for more info.