Unfreeze and update 3d_armor (split modpack) #170
Conversation
| -- | ||
| -- @section crystal | ||
|
|
||
| if armor.materials.crystal then |
There was a problem hiding this comment.
It would be best to log if it isn't found. To be fair, I haven't seen where armor.materials.crystal is set in the first place.
There was a problem hiding this comment.
If this is in init, then the materials must be checked in a dependence. This looks like an incomplete split to me. The 3d_armour API should be materials agnostic.
Though it probaby warrants an issue, it's not a show stopper for me.
There was a problem hiding this comment.
Based on https://github.com/minetest-mods/3d_armor/blob/master/3d_armor/init.lua, it looks like it's related to a legacy configuration mechanism. The given armor.conf.example has the following comment at the top:
-- DEPRECATED, will not be supported in future versions
There are no issues opened about it though.
|
Updated my review. No further concerns were noticed. Still want to (at least) make an upstream issue about that last unresolved review comment |
|
closes #169 |
|
No worries :) Thanks! |
This PR is for unfreezing 3d_armor and return to latest. This includes a major change that splits each armor into their own sub-mods inside the modpack.
By one of the maintainer's account, it has been thoroughly tested, both with automated tests and upgraded in public servers for over a year. To me this is much more solid evidence to backwards compatibility than any amount of playtesting I'd be willing to do :D
Closes #133, but not yet #169 as it has not been merged yet. We can merge the fix (trivial) later if need be, or add it here if it's merged soon.