search

mt-mods / technic

Technic mod for Minetest
18 stars 25 forks source link

Update registration functions for technic #230

Closed S-S-X closed 3 years ago

S-S-X commented 3 years ago

Closes #117

Changes excluding tests: https://github.com/mt-mods/technic/compare/master...ecea17ec9979a4e0b6a0ac4d3298398175f6c136

API

Updates machine and cable registration API to follow signature of minetest.register_node function. Allows registering from different mod and setting any mod name for nodes. Updates cable plate placement to use actual math instead of weird lookup tables. Reduces hard coded data in core source files. Reduces nested registration wrappers.

List of updated registration functions

Compatibility

There's another branch to do similar with chests, usable starting point with working API interface but incomplete implementation.

S-S-X commented 3 years ago

Any opinions on this? Acceptable for this case or do you think node name change should be reverted here?

Changed solar array naming to follow commonly used naming technic:tier_machinename. All other machines seem to follow this convention, also textures are named like this (even for solar arrays).

This makes it bit simpler which is good but also adds another alias and generally changing node name is not exactly nice thing to do.

(commit message is bit misleading: tier is removed from automatically generated node name and moved one level up)

OgelGames commented 3 years ago

I think the node name should be not altered at all (besides _active, etc. suffixes) for any of the machines.

As for the name changes, I'm all for consistency, so 👍 (I've searched for "hv_solar" way too many times...)

S-S-X commented 3 years ago

I think the node name should be not altered at all (besides _active, etc. suffixes) for any of the machines.

There's no other changes to actual node names so far, commits to remove tier is just to make caller responsible for adding tier if he wants it somewhere in name. Inside Technic those tier parts are just moved up one level up to allow free form node names.

Simply put only change to node naming so far is: Names like technic:solar_array_mv becomes technic:mv_solar_array for all solar arrays. Aliases like minetest.register_alias("technic:solar_array_mv", "technic:mv_solar_array") are added for all solar arrays.

S-S-X commented 3 years ago

Basic automated registration tests done, however part of tests are currently disabled for base machine and battery box because all fields are not yet available through definition table. Basic API interface however is done and rest of it should be mostly added feature instead of breaking changes.

Test report contains notification about this:

Pending → spec/api_spec.lua @ 139
Technic API Machine registration registers my_mod:my_battery
spec/api_spec.lua:139: Battery box registration does not include all fields

Pending → spec/api_spec.lua @ 196
Technic API Machine registration registers my_mod:machine_base
spec/api_spec.lua:196: Base machine registration does not include all fields
S-S-X commented 3 years ago

Removed WIP label as I think merging this should be decided before continuing to work on actual API features.

Changes are fairly big and some testing is needed, I've however tested this against extended-tests branch and there it passed every test case just fine. There's however no verification if all builtin machines are actually still registered or if I managed to remove some...

edit. yes, I did forgot to include tier for node names but fixed now.

edit2. if interested here's full test report: luacov.report.txt (with tests from extended-tests branch)

S-S-X commented 3 years ago

Merged master branch (extended-tests) and cleaned up commits a bit, will do another iteration later for commits that have mixed up code and tests.

S-S-X commented 3 years ago

If there's no objections I think I'll be merging this one soon.

I'll add few simple tests for overload and API to this branch before that, very soon.

Also @BuckarooBanzay opinion about renaming solar array nodes to follow style of texture names and other machine names: _technic:solar_arraymv becomes _technic:mv_solararray and similar for lv and hv other solar arrays. If that feels like bad idea it can be removed easily from this PR and even keep common style for automatic texture naming.

Started working on refactoring network and registration stuff on top of this change few days ago, mostly fixing TODO notes I've left when rewriting core network code on #96

S-S-X commented 3 years ago

Cleaned up debug prints and rebased... and I think I'll do that again to clean up first commits...

S-S-X commented 3 years ago

Tested cable plates placement using machines a bit: Constructor and deployer works fine, replacer however seems to be broken.

SwissalpS commented 3 years ago

Tested placement using machines a bit: Constructor and deployer works fine, replacer however seems to be broken.

replacer has not worked for cable plates because they don't use param values. Replacer only stores item name, param and param2.

github-actions[bot] commented 3 years ago
Click for detailed source code test coverage report ### Test coverage report for Technic CNC 79.01% in 10/14 files: ``` File Hits Missed Coverage ----------------------------------------------------- programs.lua 263 0 100.00% materials/basic_materials.lua 17 0 100.00% materials/default.lua 177 4 97.79% cnc.lua 50 3 94.34% materials/init.lua 13 1 92.86% formspec.lua 103 8 92.79% digilines.lua 39 8 82.98% init.lua 19 6 76.00% api.lua 160 83 65.84% pipeworks.lua 25 13 65.79% materials/technic_worldgen.lua 0 25 0.00% materials/moreblocks.lua 0 29 0.00% materials/ethereal.lua 0 37 0.00% materials/bakedclay.lua 0 13 0.00% ``` ### Test coverage report for technic chests 45.24% in 6/6 files: ``` File Hits Missed Coverage ---------------------------------- chests.lua 98 18 84.48% init.lua 34 18 65.38% register.lua 84 78 51.85% formspec.lua 76 93 44.97% inventory.lua 10 100 9.09% digilines.lua 2 61 3.17% ``` ### Test coverage report for technic 65.26% in 111/111 files: ``` File Hits Missed Coverage -------------------------------------------------------------- max_lag.lua 12 0 100.00% machines/register/init.lua 15 0 100.00% machines/register/freezer_recipes.lua 13 0 100.00% machines/other/init.lua 8 0 100.00% machines/MV/solar_array.lua 12 0 100.00% machines/MV/init.lua 17 0 100.00% machines/MV/grinder.lua 17 0 100.00% machines/MV/generator.lua 9 0 100.00% machines/MV/freezer.lua 17 0 100.00% machines/MV/extractor.lua 17 0 100.00% machines/MV/electric_furnace.lua 17 0 100.00% machines/MV/compressor.lua 17 0 100.00% machines/MV/centrifuge.lua 17 0 100.00% machines/MV/battery_box.lua 17 0 100.00% machines/MV/alloy_furnace.lua 19 0 100.00% machines/LV/solar_array.lua 11 0 100.00% machines/LV/init.lua 17 0 100.00% machines/LV/grinder.lua 16 0 100.00% machines/LV/generator.lua 9 0 100.00% machines/LV/electric_furnace.lua 15 0 100.00% machines/LV/compressor.lua 20 0 100.00% machines/LV/battery_box.lua 15 0 100.00% machines/LV/alloy_furnace.lua 17 0 100.00% machines/HV/solar_array.lua 11 0 100.00% machines/HV/init.lua 12 0 100.00% machines/HV/grinder.lua 17 0 100.00% machines/HV/generator.lua 9 0 100.00% machines/HV/electric_furnace.lua 17 0 100.00% machines/HV/compressor.lua 17 0 100.00% machines/HV/battery_box.lua 17 0 100.00% legacy.lua 33 0 100.00% items.lua 128 0 100.00% crafts.lua 133 0 100.00% ../technic_worldgen/nodes.lua 109 0 100.00% ../technic_worldgen/crafts.lua 103 0 100.00% ../technic_worldgen/config.lua 9 0 100.00% ../technic_cnc/programs.lua 263 0 100.00% ../technic_cnc/materials/technic_worldgen.lua 32 0 100.00% ../technic_cnc/materials/init.lua 14 0 100.00% ../technic_cnc/materials/basic_materials.lua 17 0 100.00% machines/LV/led.lua 73 1 98.65% ../technic_cnc/materials/default.lua 178 4 97.80% machines/register/cables.lua 174 4 97.75% machines/register/compressor_recipes.lua 36 1 97.30% machines/LV/geothermal.lua 75 3 96.15% config.lua 49 2 96.08% machines/register/solar_array.lua 46 2 95.83% machines/LV/solar_panel.lua 42 2 95.45% ../technic_cnc/cnc.lua 50 3 94.34% tools/init.lua 13 1 92.86% machines/network.lua 327 27 92.37% machines/LV/water_mill.lua 67 6 91.78% machines/register/grindings.lua 42 5 89.36% machines/LV/lamp.lua 110 14 88.71% ../technic_worldgen/overrides.lua 39 5 88.64% machines/MV/cables.lua 29 4 87.88% machines/LV/cables.lua 29 4 87.88% init.lua 29 4 87.88% machines/HV/cables.lua 28 4 87.50% machines/register/grinder_recipes.lua 106 16 86.89% ../technic_worldgen/rubber.lua 64 10 86.49% ../technic_worldgen/init.lua 19 3 86.36% ../technic_worldgen/oregen.lua 155 28 84.70% util/throttle.lua 9 2 81.82% machines/register/machine_base.lua 165 41 80.10% ../technic_cnc/formspec.lua 88 23 79.28% machines/LV/extractor.lua 18 5 78.26% ../technic_cnc/init.lua 19 6 76.00% machines/register/recipes.lua 62 20 75.61% tools/flashlight.lua 65 21 75.58% machines/switching_station.lua 76 25 75.25% machines/register/battery_box.lua 222 73 75.25% machines/register/centrifuge_recipes.lua 21 7 75.00% radiation.lua 262 88 74.86% ../technic_cnc/api.lua 194 66 74.62% effects.lua 5 2 71.43% machines/MV/wind_mill.lua 46 22 67.65% machines/other/coal_furnace.lua 2 1 66.67% machines/supply_converter.lua 93 48 65.96% ../technic_cnc/pipeworks.lua 25 13 65.79% machines/switching_station_globalstep.lua 40 21 65.57% machines/register/alloy_recipes.lua 28 15 65.12% machines/other/injector.lua 72 39 64.86% machines/MV/hydro_turbine.lua 43 26 62.32% machines/MV/tool_workshop.lua 56 34 62.22% machines/register/generator.lua 122 88 58.10% register.lua 21 19 52.50% tools/cans.lua 53 48 52.48% machines/power_monitor.lua 40 38 51.28% machines/init.lua 55 54 50.46% machines/other/coal_alloy_furnace.lua 63 63 50.00% machines/LV/music_player.lua 46 46 50.00% tools/mining_lasers.lua 39 40 49.37% machines/other/constructor.lua 67 69 49.26% tools/vacuum.lua 19 21 47.50% tools/tree_tap.lua 24 27 47.06% machines/HV/forcefield.lua 101 157 39.15% machines/HV/quarry.lua 124 217 36.36% machines/HV/nuclear_reactor.lua 116 208 35.80% machines/register/common.lua 39 75 34.21% tools/chainsaw.lua 40 83 32.52% tools/multimeter.lua 71 149 32.27% machines/compat/api.lua 16 34 32.00% tools/sonic_screwdriver.lua 18 40 31.03% machines/other/frames.lua 184 445 29.25% helpers.lua 38 100 27.54% tools/mining_drill.lua 76 226 25.17% machines/other/anchor.lua 14 74 15.91% machines/compat/digtron.lua 2 11 15.38% tools/prospector.lua 16 92 14.81% machines/register/extractor_recipes.lua 6 65 8.45% ``` ### Raw test runner output for geeks: CNC: ``` ●●●●●●●●●●●●●●●●●●●●●● 22 successes / 0 failures / 0 errors / 0 pending : 0.171042 seconds ``` Chests: ``` W: Configuration: invalid key exclude_textures W: Configuration: invalid key validate_textures ●●●●● 5 successes / 0 failures / 0 errors / 0 pending : 0.038228 seconds ``` Technic: ``` ●◌◌●●●◌●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●◌●●●●● 174 successes / 0 failures / 0 errors / 4 pending : 6.893 seconds Pending → spec/api_spec.lua @ 132 Technic API Machine registration registers my_mod:my_battery spec/api_spec.lua:132: Battery box registration does not include all fields Pending → spec/api_spec.lua @ 189 Technic API Machine registration registers my_mod:machine_base spec/api_spec.lua:189: Base machine registration does not include all fields Pending → spec/api_spec.lua @ 284 Technic API internals technic.cables TBD, misleading name and should be updated spec/api_spec.lua:284: TBD technic.cables naming and need, see technic networks data for possible options Pending → spec/supply_converter_spec.lua @ 78 Supply converter building overloads network spec/supply_converter_spec.lua:78: overload does not work with supply converter ```
S-S-X commented 3 years ago

Did some little play testing, jumpdrive and space cannons were fine so machine registration compatibility is tested. Screwdriver was not fine with cable plates because for some reason I had paramtype2 = "facedir", fixed that.