Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update registration functions for technic #230

Merged
merged 5 commits into from
Nov 15, 2021
Merged

Update registration functions for technic #230

merged 5 commits into from
Nov 15, 2021

Conversation

S-S-X
Copy link
Member

@S-S-X S-S-X commented Oct 28, 2021

Closes #117

Changes excluding tests: master...ecea17e

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.

  • Test cable plate placement using machines and tools.
    • Either one time manual test or automated tests, does not really matter.
  • Test for registrations with different mod name and different current mod name reported by engine.
    • Should be automated test to allow verifying API compatibility.

List of updated registration functions

  • New signature: function technic.register_base_machine(name, def)
    • Was: function technic.register_base_machine(data)
  • New signature: function technic.register_solar_array(name, def)
    • Was: function technic.register_solar_array(data)
  • New signature: function technic.register_battery_box(name, def)
    • Was: function technic.register_battery_box(data)
  • New signature: function technic.register_cable(nodename, data)
    • Was: function technic.register_cable(tier, size, description, prefix, override_cable, override_cable_plate)
    • It was not possible to easily use these outside of technic mod, there should not be any conflicts.
  • New function: function technic.register_cable_plate(nodename, data)
    • Cable plates were always registered through technic.register_cable, added new function for this.
    • It was not possible to easily use these outside of technic mod, there should not be any conflicts.

Compatibility

  • Machine registration API follows minetest.register_node interface:
  • Machine registration is compatible with old style interface
    • see technic/machines/compat/api.lua
    • Logs warning message when old function signature is used. Data is upgraded and machine registered.
    • Does not modify input data, for some parts this is different from previous implementation.
    • Can be changed to modify input data for very small performance increase. However I don't see this being important at all for registration (server startup time), difference is very small and makes API usage harder.
  • Old machine registration wrappers moved to technic/machines/compat/api.lua:
    • function technic.register_alloy_furnace(def)
    • function technic.register_centrifuge(def)
    • function technic.register_compressor(def)
    • function technic.register_extractor(def)
    • function technic.register_freezer(def)
    • function technic.register_grinder(def)
    • function technic.register_electric_furnace(def)
    • Register these directly without using wrappers.
    • Add warning logging if functions are used.

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

@S-S-X S-S-X added Enhancement New feature or request WIP Work in progress labels Oct 28, 2021
@S-S-X
Copy link
Member Author

S-S-X commented Oct 29, 2021

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
Copy link
Contributor

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
Copy link
Member Author

S-S-X commented Oct 30, 2021

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
Copy link
Member Author

S-S-X commented Oct 30, 2021

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 S-S-X removed the WIP Work in progress label Oct 30, 2021
@S-S-X
Copy link
Member Author

S-S-X commented Oct 30, 2021

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)

@OgelGames OgelGames added this to the 2.0.0 milestone Oct 30, 2021
OgelGames
OgelGames previously approved these changes Oct 30, 2021
@OgelGames OgelGames dismissed their stale review October 30, 2021 13:42

wrong PR

@S-S-X S-S-X added the Cleanup Cleanup of bad code or other redundant/unnecessary stuff label Nov 5, 2021
@S-S-X
Copy link
Member Author

S-S-X commented Nov 6, 2021

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 S-S-X force-pushed the issue-117 branch 2 times, most recently from db907b7 to 81ce564 Compare November 10, 2021 09:44
@S-S-X
Copy link
Member Author

S-S-X commented Nov 12, 2021

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_array_mv becomes technic:mv_solar_array 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
Copy link
Member Author

S-S-X commented Nov 12, 2021

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

@S-S-X
Copy link
Member Author

S-S-X commented Nov 12, 2021

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

@SwissalpS
Copy link
Contributor

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
Copy link

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
Copy link
Member Author

S-S-X commented Nov 12, 2021

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.

@S-S-X S-S-X mentioned this pull request Nov 13, 2021
Copy link
Contributor

@OgelGames OgelGames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard to check every line of code, but looking at the main changes, everything seems to be good 👍

I also loaded a world with the pandorabox_integration_test enabled, it didn't complain about anything either :)

@S-S-X S-S-X merged commit 5aeeb27 into master Nov 15, 2021
@OgelGames OgelGames deleted the issue-117 branch November 25, 2021 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Cleanup of bad code or other redundant/unnecessary stuff Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite all machine/cable registration functions to have generic format
3 participants