Fix and cleanup dependencies #224
Conversation
OgelGames
added
WIP
CNC does not require technic and it has been used on some servers without technic. |
|
Ah, I didn't realize that it was functional without technic. Still has one thing to fix though, when |
|
I think it would make sense to divide There's even |
|
It also seems like it would be easy to make also dependency to Just make registered nodes table empty Line 26 in b2f456d And use API method to register all those nodes if default is loaded Lines 67 to 71 in b2f456d |
Mineunit failed regression tests, click for detailsRegression test log for technic: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This might not be wise thing to do because every unnecessary registration check during register_on_mods_loaded makes system as a whole lot more complicated and is potential source of problems in future (not as bad but similar to using minetest.after to "fix" bugs). For many things optional dependencies are good to have if there's real feature dependency, not only because it will allow Minetest to do suggestions for mods and ContentDB to show things that are meant to work together but also because it makes loading order clear and visible. Is there some real reason (mod conflict) that requires dropping optional dependencies? |
No, the main reason is because they aren't dependencies at all, they're just there so some items can be renamed. This is the code I was thinking of changing, using technic/technic_worldgen/crafts.lua Lines 146 to 194 in 2add428 And also this: technic/technic_worldgen/nodes.lua Lines 156 to 196 in 2add428 Yes it doesn't need to be changed, but I think it should be, and also be moved to a separate file. |
S-S-X
force-pushed
the
master
branch
2 times, most recently
from
ef88bf1 to
e5e4cbe
Compare
In that case it could make sense, only reason to keep optional dependencies in mod.conf would be to make it visible that mod does something with another mod. Probably not that important for small changes done by technic_worldgen. I did take a look at the code and one thing made me wonder if it works or breaks things: Anyway, that cannot be really fixed here if translations wont work after editing descriptions but I think it is something that should be considered and tested. If modified translations do break (or if translations are not tested) then would be good to open separate issue about that and let it be broken for now. |
|
Yeah, I think translations were broken before, and they will still be broken after these changes. Probably still a good idea to open an issue about it, or at least mention it in #104 |
removed dependency on `screwdriver` and added `moreores` dependency
OgelGames
force-pushed
the
depends-cleanup
branch
from
ed4263b to
9cb0518
Compare
|
Tidied up the commits, so the different changes can be merged as different commits. (rebase merge) I don't think there is anything else that needs to be done in this PR, so it should be ready for merging (after an approval is added). |
reduces optional dependencies and extends to support as many items as possible
OgelGames
force-pushed
the
depends-cleanup
branch
from
9cb0518 to
9ceb4e6
Compare
Click for detailed source code test coverage reportTest coverage report for Technic CNC 78.96% in 10/14 files:Test coverage report for technic 9.61% in 10/103 files:Raw test runner output for geeks:CNC: Technic: |
depends.txtfilestechnic_worldgenusingminetest.register_on_mods_loaded()Make CNC depend ontechnic(currently optional, which doesn't make sense)Not including #214 in this PR because that will be a much bigger change, as opposed these small changes and fixes.