Closed Panquesito7 closed 4 years ago
Hi, I don't have the time to review properly the mods/3d_armor/3d_armor/armor.lua
part
It's the tricky part where we need to keep the specific MFF code parts and merge them with the new mod code
Hi, I don't have the time to review properly the
mods/3d_armor/3d_armor/armor.lua
part. It's the tricky part where we need to keep the specific MFF code parts and merge them with the new mod code
Well, I just saw some comments in the code that it was modified.
But, some code in the current version of 3d_armor
was split in other files.
So, it's more hard than it looks. I'll continue to update all the other mods.
I've completed updating all mods (excluding out-dated and modified mods).
I have no idea how to keep the MFF changes in 3d_armor
. Help welcome.
@MinetestForFun/devs, please test and review this PR. Your thoughts/feedback would be appreciated.
Looks great! So happy to see some activity on this repo. I will probably help testing, too.
Yes that's great, thank you for your work
But, I don't have time to merge the old MFF tweaked code with the new one in 3D_armor sorry
I can't accept the merge request until this is fixed
But, I don't have time to merge the old MFF tweaked code with the new one in 3D_armor sorry
I can fix it as well; I just don't know which files I should update.
armor.lua
was 99% re-writed. It registers only the armors.
The API is found at api.lua
.
Other stuff such as the armor materials are at init.lua
.
EDIT: I have updated 3d_armor
and 3d_armor_classes.
Please review it.
Looks great! So happy to see some activity on this repo. I will probably help testing, too.
Thank you, @Gael-de-Sailly, for your help. 👍
As is, this pull request needs a lot of work, mostly related to inventory access api on the part of 3d_armor. Mods like bones or pclasses cannot read the armor inventory right now. Crashes such as this one happen when I join and try to play a bit
Runtime error from mod 'bones' in callback on_dieplayer(): ...minetest/.minetest/worlds/world/worldmods/bones/init.lua:208: attempt to index local 'armor_inv' (a nil value)
Runtime error from mod 'pclasses' in callback on_joinplayer(): ...netest/.mine
test/worlds/world/worldmods/pclasses/api.lua:147: attempt to index local 'inv' (a nil value)
Would you like to investigate these on your own?
EDIT: I'd also like to add that I'm testing this server in version 0.5.0.1, meaning that if this PR goes through, we can upgrade MFF CL to 0.5.0.1
EDIT2: I went ahead and made some of the changes needed for the server not to crash. I compiled them into a commit that you can cherry-pick/patch : 083780e. I haven't seen anything explicitly wrong with your update, so you've done a good job. Good luck!
we can upgrade MFF CL to 5.0.1
In order to get this PR merged, the server must be updated to 5.0.1, otherwise, it won't work.
I went ahead and made some of the changes needed for the server not to crash
Thank you, @LeMagnesium for your help, work, and review. It is greatly appreciated. I applied your changes.
- A failed dependency from the
paintings
mod
I do not see dependencies issues on paintings
.
It should work well.
- Various crashes related to 3d_armor inventory access
Where are located those files that create crashes? AFAIK, I think there are no more crashes.
I do not see dependencies issues on paintings. It should work well.
Well, the sound of painting nodes is obtained from default
(see this).
Where are located those files that create crashes?
Every file that needed fixes was modified in my own commit here.
EDIT: I'll go over the changes needed in the review of this PR. EDIT2: I see you've incorporated my changes. As is, it will boot. There are still things that need to be tweaked a bit.
I'll stay online for a bit while you apply the last modifications. When they're done, I'll approve the PR.
Well, the sound of painting nodes is obtained from
default
(see this).
paintings
has a hard dependency on default
already.
When they're done, I'll approve the PR.
Done.
Done.
Almost. Keep it up, you're almost there. You just need to reinstate all the shield stats and change one more instance of "moreores" into "default" and then I'll approve it and let Darcidride decide.
Almost. Keep it up, you're almost there.
I think it's done now. 🙂
I'm pleasantly surprised that things are going so fast. I didn't even have the time to help reviewing XD (was busy this weekend).
However, about upgrading to 5.0, I spoke with Leagdar yesterday, and he told me he won't be able to switch to 5.0 (because old computer). I don't know how many players are in this case, but I suspect most of our players come here because they're stuck on 0.4.
I don't want to block the update, but this should be considered.
I'm pleasantly surprised that things are going so fast. I didn't even have time to help reviewing XD (was busy this weekend).
😄
and he told me he won't be able to switch to 5.0 (because old computer).
EDIT: Which operating system do they have? If they minimum have at least C++11 (and Windows Vista), they can update the server to 5.0.1.
most of our players come here because they're stuck on 0.4.
I actually before wanted to join the server, but then, I realized it was 0.4.
I don't want to block the update, but this should be considered.
Thanks for the information, @Gael-de-Sailly.
Just as I approved the code, I noticed the following crash when testing.
Runtime error from mod '??' in callback detached_inventory_AllowPut(): ...netest/worlds/world/worldmods/3d_armor/3d_armor/init.lua:192: attempt to index global 'stack' (a nil value)
Try putting armor on while playing the game, and this should happen.
I noticed the following crash when testing.
Fixed.
A last crash arose during my testing of 3d_armor
.
Runtime error from mod '3d_armor' in callback environment_Step(): ...inetest/worlds /world/worldmods/3d_armor/3d_armor/api.lua:226: attempt to index global 'item' (a nil value)
This one is a simple fix, and in my quick and dirty testing of 3d_armor
, it's the last crash I encountered. Just replace item
by stack:get_name()
on the line of the crash.
A last crash arose during my testing of
3d_armor
. ...
Sorry for the long delay; fixed.
It's fine for me.
Good, thank you. I'll wait for the approval of the other developers/members.
@LeMagnesium, @Darcidride, really thanks for your reviews, it is greatly appreciated! 🙂
@Panquesito7 You can merge it
For the update in production, we will need to sync someday, in case we met issues
Removes deprecated functions, replace
depends.txt
anddescription.txt
withmod.conf
, update3d_armor
to the latest version (0.4.13).Also updates various other mods. Excludes some (very) out-dated and modified mods.
TODO
3d_armor
.